Skip to content

converted the old tailwind css example to typescript #32808

Merged
kodiakhq[bot] merged 4 commits intovercel:canaryfrom
kumard3:canary
Jan 5, 2022
Merged

converted the old tailwind css example to typescript #32808
kodiakhq[bot] merged 4 commits intovercel:canaryfrom
kumard3:canary

Conversation

@kumard3
Copy link
Copy Markdown
Contributor

@kumard3 kumard3 commented Dec 25, 2021

Bug

  • Related issues linked using fixes #number
  • Integration tests added
  • Errors have helpful link attached, see contributing.md

Feature

  • Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
  • Related issues linked using fixes #number
  • Integration tests added
  • Documentation added
  • Telemetry added. In case of a feature if it's used or not.
  • Errors have helpful link attached, see contributing.md

Documentation / Examples

  • Make sure the linting passes by running yarn lint

@ijjk ijjk added the examples Issue was opened via the examples template. label Dec 25, 2021
balazsorban44
balazsorban44 reviewed Dec 25, 2021
View reviewed changes
Comment thread examples/with-tailwindcss/package.json Outdated
balazsorban44
balazsorban44 reviewed Dec 25, 2021
View reviewed changes
Comment thread examples/with-tailwindcss/tailwind.config.js Outdated
balazsorban44
balazsorban44 suggested changes Dec 25, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@balazsorban44 balazsorban44 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

I left some comments.

Could you also please run npx prettier --write ./examples/with-tailwindcss to fix the lint errors and commit the changes?

@balazsorban44 balazsorban44 mentioned this pull request Dec 26, 2021
Closed
@kumard3
Copy link
Copy Markdown
Contributor Author

kumard3 commented Dec 26, 2021

Thanks.

I left some comments.

Could you also please run npx prettier --write ./examples/with-tailwindcss to fix the lint errors and commit the changes?

Yes sure will be doing that

@balazsorban44
Copy link
Copy Markdown
Contributor

balazsorban44 commented Dec 26, 2021

Please let's focus this PR on the tailwind example. If you would like to convert other examples to TypeScript, you are welcome to do so, but open a separate PR instead. It makes it easier to track down changes in the future. Thank you.

@kumard3
Copy link
Copy Markdown
Contributor Author

kumard3 commented Dec 27, 2021

Please let's focus this PR on the tailwind example. If you would like to convert other examples to TypeScript, you are welcome to do so, but open a separate PR instead. It makes it easier to track down changes in the future. Thank you.

Sure will we doing that, and from on I will be doing this in a systemic way.

leerob
leerob reviewed Jan 4, 2022
View reviewed changes
Comment thread examples/progressive-web-app/pages/api/hello.js Outdated
@kumard3
Copy link
Copy Markdown
Contributor Author

kumard3 commented Jan 4, 2022

@leerob I have reverted all the unrelated files

balazsorban44
balazsorban44 suggested changes Jan 4, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@balazsorban44 balazsorban44 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove all the changes, except in the folder: examples/with-tailwindcss as requested here #32808 (comment) (I could have been more explicit)

There are still changes within examples/progressive-web-app

@ijjk ijjk added create-next-app Related to our CLI tool for quickly starting a new Next.js application. area: documentation type: next labels Jan 4, 2022
@kumard3 kumard3 force-pushed the canary branch 2 times, most recently from bc41514 to ddd1678 Compare January 4, 2022 18:46
@kumard3
Copy link
Copy Markdown
Contributor Author

kumard3 commented Jan 4, 2022

Please remove all the changes, except in the folder: examples/with-tailwindcss as requested here #32808 (comment) (I could have been more explicit)

There are still changes within examples/progressive-web-app

I have reverted the commit , but it is still showing the same changes in file, can you please guide me , to performer this task.

@kumard3 kumard3 force-pushed the canary branch 2 times, most recently from 8902b2d to 5585bf2 Compare January 4, 2022 22:16
@kumard3
Copy link
Copy Markdown
Contributor Author

kumard3 commented Jan 4, 2022

@balazsorban44 I have reverted all the unrelated files .

@kumard3 kumard3 requested a review from balazsorban44 January 4, 2022 22:41
@kumard3 kumard3 requested a review from leerob January 5, 2022 02:45
@kodiakhq kodiakhq Bot merged commit 066984e into vercel:canary Jan 5, 2022
teleaziz added a commit to teleaziz/next.js that referenced this pull request Jan 5, 2022
@teleaziz
Merge branch 'canary' of github.com:vercel/next.js into cms-builder-i…
c9fadc8 
…o-example

* 'canary' of github.com:vercel/next.js:
  [chore] Update `deta` version in examples (vercel#30204)
  fix: typescript example supporting strict w/ version >= 4.4 (vercel#33042)
  Avoid page double render with emotion vanilla (vercel#30541)
  converted the old tailwind css example to typescript  (vercel#32808)
  fix(examples/cms-contentful): add correct Content-Type + missing closing tag for html (vercel#30321)
  Ensure NODE_ENV is not inlined for next/jest (vercel#33032)
  Rename api in with-redis example (vercel#33016)
  Remove unused turbo remote cache env vars (vercel#33030)
  v12.0.8-canary.17
  Re-enable turbo caching for swc build jobs (vercel#32617)
  feat(cli): introduce `next info` CLI command (vercel#32972)
  fix(examples): add missing dependencies wo (vercel#32977)
  Updated wrong link to example of gtag init in measuring-performance.md (vercel#32974)
  v12.0.8-canary.16
  Revert "Reduce install size for linux glibc/musl (vercel#32850)" (vercel#32973)
  Ensure middleware is output in standalone mode (vercel#32967)
  v12.0.8-canary.15
  Reduce install size for linux glibc/musl (vercel#32850)
  Fixes issue with makeStylesheetInert (vercel#32027)
  Ensure setImmediate and punycode are polyfilled (vercel#32768)
leerob
leerob reviewed Jan 6, 2022
View reviewed changes
Comment thread examples/with-tailwindcss/.eslintrc.json
@@ -0,0 +1,3 @@
{
"extends": "next/core-web-vitals"
Copy link
Copy Markdown
Contributor

@leerob leerob Jan 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should include ESLint - while it is helpful, I think this example should be as simple as possible - just Next.js and Tailwind.

leerob
leerob reviewed Jan 6, 2022
View reviewed changes
Comment thread examples/with-tailwindcss/README.md
@@ -1,3 +1,11 @@
<p align="center">

<img src="https://res.cloudinary.com/ddcg0rzlo/image/upload/v1640340715/nextjs-tailwind-typescript-banner_vslgq4.png" alt="Next.js TypeScript Starter">
Copy link
Copy Markdown
Contributor

@leerob leerob Jan 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we remove this image please?

@balazsorban44
Copy link
Copy Markdown
Contributor

balazsorban44 commented Jan 6, 2022 •
edited
Loading

@kumard3 could you please follow up in a separate PR with @leerob's suggestion?

#33062 addressed this.

@LazirusDon
Copy link
Copy Markdown

LazirusDon commented Jan 7, 2022

example is typescript now? how do i start a nextjs tailwind without typescript ?
how to initialize next/tailwind with just js?

npx create-next-app --example with-tailwindcss with-tailwindcss-app
converted the old tailwind css example to typescript (#32808),

@leerob
Copy link
Copy Markdown
Contributor

leerob commented Jan 7, 2022

Yes, the example is now using TypeScript. If you would prefer not to use it, you can remove the TS configuration (it's a few files/lines).

@LazirusDon
Copy link
Copy Markdown

LazirusDon commented Jan 7, 2022 via email

@ahmadbilaldev
Copy link
Copy Markdown

ahmadbilaldev commented Jan 8, 2022 •
edited
Loading

Why was the JavaScript example changed to TypeScript? I think the default example should be using JavaScript. I refer this in my next app guides and it can be complicated for the readers to manually convert TS to JS.

We can do something like -with-ts-tailwindcss for the TypeScript instead.

This was referenced Jan 12, 2022
Open
Open
Open
Open
Open
Closed
Open
@snyk-bot snyk-bot mentioned this pull request Jan 21, 2022
Open
@msaaddev
Copy link
Copy Markdown

msaaddev commented Jan 27, 2022 •
edited
Loading

@ahmadbilaldev totally agrees with you here. There should also be a JavaScript-based template available for the Next.js TailwindCSS example.

@balazsorban44 can we do something about it?

@balazsorban44
Copy link
Copy Markdown
Contributor

balazsorban44 commented Jan 28, 2022 •
edited
Loading

The way forward is that we are going to focus on TypeScript and prefer it over JS in examples, as it provides more safety even for beginner developers, and is being more and more widely adopted.

We are discussing alternatives like a --js flag that could potentially harness SWC to automatically convert examples to plain JavaScript, but nothing is set yet. It is easier to go from TypeScript to JavaScript, not the other way around.

@balazsorban44 balazsorban44 mentioned this pull request Feb 20, 2022
Closed
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Feb 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@ijjk ijjk ijjk approved these changes

@shuding shuding Awaiting requested review from shuding

@steven-tey steven-tey Awaiting requested review from steven-tey

@timneutkens timneutkens Awaiting requested review from timneutkens

@huozhi huozhi Awaiting requested review from huozhi

@padmaia padmaia Awaiting requested review from padmaia

@styfle styfle Awaiting requested review from styfle

+2 more reviewers

@leerob leerob leerob left review comments

@balazsorban44 balazsorban44 balazsorban44 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

create-next-app Related to our CLI tool for quickly starting a new Next.js application. examples Issue was opened via the examples template. type: next

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@kumard3 @balazsorban44 @LazirusDon @leerob @ahmadbilaldev @msaaddev @ijjk