FredKSchott / snowpack

ESM-powered frontend build tool. Instant, lightweight, unbundled development. ✌️
https://www.snowpack.dev
MIT License
19.48k stars 921 forks source link

[BUG] Disconcerting warnings and errors in blank Typescript template #3220

Open richb-hanover opened 3 years ago

richb-hanover commented 3 years ago

Bug Report Quick Checklist

Describe the bug

The blank Typescript template gives several warnings/error messages. This is disconcerting for people who are new to Snowpack, and just trying it out...

To Reproduce

Read the README.md for details, but the outline is:

  1. npx create-snowpack-app test26mar --template @snowpack/app-template-blank-typescript --use-yarn
  2. see several deprecation warnings...
  3. s/3.0.1/3.3.4/ in package.json
  4. Deprecations go away when re-running yarn
  5. yarn start gives a Rollup preventAssignment error even though Rollup is not (obviously) being used

Expected behavior

No errors or warnings from the npx or yarn start commands

Anything else?

Demo repo and README.md at: https://github.com/richb-hanover/SnowpackTest-26Apr

IanVS commented 3 years ago

This seems to be a duplicate of https://github.com/snowpackjs/snowpack/issues/3151.

richb-hanover commented 3 years ago

Not quite. I agree that #3151 talks about the "preventAssignment" error message.

But the "blank Typescript" template also generates warnings. (I believe that switching from Snowpack 3.0.1 to 3.3.4 solves this.) Thanks.

fgblomqvist commented 3 years ago

Could you paste the logs to show which warnings/errors you mean?

Also, rollup is used for bundling dependencies so yes, it is used 🙂 .

richb-hanover commented 3 years ago

The logs are in the README at: https://github.com/richb-hanover/SnowpackTest-26Apr and reproduced here...

Starting new Snowpack project

√ github % npx create-snowpack-app test26mar --template @snowpack/app-template-blank-typescript --use-yarn
npx: installed 24 in 2.795s

  - Using template @snowpack/app-template-blank-typescript
  - Creating a new project in /Users/richb/github/test26mar
  - Installing package dependencies. This might take a couple of minutes.

warning package.json: No license field
warning ../package.json: No license field
warning No license field
warning snowpack > rollup > fsevents@2.1.3: "Please update to latest v2.3 or v2.2"
warning snowpack > pacote > @npmcli/run-script > node-gyp > request@2.88.2: request has been deprecated, see https://github.com/request/request/issues/3142
warning snowpack > pacote > @npmcli/run-script > node-gyp > request > har-validator@5.1.5: this library is no longer supported

  - Initializing git repo.

  - Success!

Quickstart:

  cd test26mar
  yarn start

All Commands:

  yarn install     Install your dependencies. (We already ran this one for you!)
  yarn start       Start your development server.
  yarn run build   Build your website for production.
  yarn test        Run your tests.

Error on initial start

√ test26mar % yarn start
yarn run v1.22.10
warning package.json: No license field
warning ../package.json: No license field
$ snowpack dev
[22:14:17] [snowpack] Welcome to Snowpack! Because this is your first time running
this project, Snowpack needs to prepare your dependencies. This is a one-time step
and the results will be cached for the lifetime of your project. Please wait...
[22:14:17] [snowpack] + canvas-confetti@1.4.0
[22:14:17] [esinstall:canvas-confetti] @rollup/plugin-replace: 'preventAssignment' currently defaults to false. It is recommended to set this option to `true`, as the next major version will default this option to `true`.
[22:14:17] [snowpack] Ready!
[22:14:17] [snowpack] Server started in 16ms.
[22:14:17] [snowpack] Local: http://localhost:8080
[22:14:17] [snowpack] Network: http://192.168.253.116:8080
[22:14:18] [@snowpack/plugin-typescript] 9:13:18 PM - Starting compilation in watch mode...
[22:14:19] [@snowpack/plugin-typescript] 9:13:19 PM - Found 0 errors. Watching for file changes.
⠏ watching for file changes...^C
fgblomqvist commented 3 years ago

Looks like the initial start error is the one mentioned in #3151. The missing license warning is odd since there is clearly a license in the template you're using 🤔.

If you walk down the dep tree and check their repos you'll find this: https://github.com/npm/run-script/issues/25 (check the linked PRs for commentary) In short: there is nothing Snowpack can do about that, it'll get fixed once npmcli/run-script updates to node-gyp 8.

Finally, you'll also find that Snowpack needs to update their rollup version to get rid of the fsevents warning.

With this little research that took me 10 minutes it was possible to figure out:

  1. What snowpack can and can't fix
  2. What exactly snowpack needs to fix

Which ultimately makes it easier for the maintainer. Now, you can probably submit a PR that updates the rollup version if you wanna help 😃

richb-hanover commented 3 years ago

Thank you for tracking all these items down. I appreciate that effort. Your analysis makes sense

I am enough of a "techie" to see the power of Snowpack. And I can muddle through. But I also remain a "naive user", and see the bumps along the road - these will slow its adoption. (That's why I put "Disconcerting warnings..." in the title. They're not necessarily bugs, but they make a newcomer wonder if something bad happened, or if, perhaps, the whole project is unreliable.)

I can see you're deeply knowledgeable about the Javascript ecosystem and with the Snowpack environment. But it would take me far longer to reach those same conclusions. (And longer, still, to actually make any PRs.)

So I point out these problems a) to ask if I'm "doing it wrong" or b) to show where Snowpack could be improved. Is there a better (more helpful) place to make these observations? Could these reports be improved?

Many thanks.

fgblomqvist commented 3 years ago

Any feedback is better than no feedback, but yeah if I were to give any advice it would probably be to split the report up by best-effort in the future :)

E.g. one thing for the runtime error (that happened to be filed already), one for the package.json warnings, and one for the deprecated dependencies. That way it's easy to track them all separately. But yeah, still thanks for reporting!

richb-hanover commented 3 years ago

Excellent. I'll separate them in the future.

NB: The license warning message is probably correct. You wrote:

The missing license warning is odd since there is clearly a license in the template you're using 🤔.

But the generated package.json file (see my test repo) does not contain a "license" property. This is likely correct, since it is up to the developer to select a license for the newly-created project. The continued warnings remind me to select one.

Thanks again!

richb-hanover commented 3 years ago

@drwpow I just tried the same experiment as above with snowpack 3.8.2 (creating a blank Typescript project with CSA) and got substantially similar errors. rollup and @npmcli/arborist seem to call on deprecated, outdated, or unsupported modules. The resulting project seems to work fine, but the error messages are still disconcerting.

I'm not sure these are relevant, but I note that:

I hope this is helpful. Here's the full output

√ github % npx create-snowpack-app snowpack382 --template @snowpack/app-template-blank-typescript --use-yarn
npx: installed 24 in 2.552s

  - Using template @snowpack/app-template-blank-typescript
  - Creating a new project in /Users/richb/github/snowpack382
  - Installing package dependencies. This might take a couple of minutes.

warning package.json: No license field
warning ../package.json: No license field
warning No license field
warning snowpack > rollup > fsevents@2.1.3: "Please update to latest v2.3 or v2.2"
warning snowpack > @npmcli/arborist > @npmcli/run-script > node-gyp > request@2.88.2: request has been deprecated, see https://github.com/request/request/issues/3142
warning snowpack > @npmcli/arborist > @npmcli/run-script > node-gyp > request > har-validator@5.1.5: this library is no longer supported
warning snowpack > @npmcli/arborist > @npmcli/run-script > node-gyp > request > uuid@3.4.0: Please upgrade  to version 7 or higher.  Older versions may use Math

  - Initializing git repo.

  - Success!

Quickstart:

  cd snowpack382
  yarn start

All Commands:

  yarn install     Install your dependencies. (We already ran this one for you!)
  yarn start       Start your development server.
  yarn run build   Build your website for production.
  yarn test        Run your tests.

√ github % 
richb-hanover commented 3 years ago

Drat. We're stuck for a while. See: https://github.com/npm/cli/issues/3525#issuecomment-878290633

But maybe you can fix the rollup dependency...

drwpow commented 3 years ago

Thanks for checking back in on this. The Rollup dependency version is locked because of a pretty big bug that was introduced in a minor release, that still hasn’t been fixed: https://github.com/snowpackjs/snowpack/pull/2572. Updating Rollup to latest counterintuitively hurts our package support (if you ever see a ~ like in snowpack/package.json, it’s usually for a reason).

As for the Arborist <> node-gyp issue… yeah I’m also not too crazy about having that dependency be in Snowpack. I’d like to find a way to keep a lot of smart dependency tree parsing that Snowpack does, without incurring some of the slowness and weirdness of Arborist.

richb-hanover commented 3 years ago

OK. Now we all know that it has been checked. (My company's unofficial slogan was, "We promise not to fix it if we don't know it's broke" :-)