search

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] Inconsistent streaming package results between skypack and snowpack devserver #3393

Open JaredReisinger opened 3 years ago

JaredReisinger commented 3 years ago

Bug Report Quick Checklist

Describe the bug

When using streaming imports, importing @urql/core (v2.1.2) into my app results in a failing dependency import for -/@urql/core@v2.1.2-BaGVb5xQ5BNV3TUNPHgq/dist=es2020,mode=imports/optimized/common/5adc38f0-46a7d8df.js; instead of the expected file, I get back Could not find resource exports/common@5adc38f0-46a7d8df.js.

To Reproduce

  1. npm init snowpack-app --template @snowpack/app-template-react-typescript
  2. npm uninstall react react-dom @types/react @types/react-dom
  3. update all dependencies to latest versions (init snowpack-app seemed to install snowpack ^3.3.7, so I force-updated to 3.5.2 just to be sure)
  4. configure snowpack.config for streaming and typescript (packageOptions: { source: 'remote', types: true })
  5. npx snowpack add react
npx snowpack add react-dom
npx snowpack add @urql-core
  6. add 
    import { createClient } from '@urql/core';
const x = createClient({ url: '' });
console.log(x);

    to src/App.tsx

  7. run npm start (a.k.a. npx snowpack dev)

  8. result: HMR overlay error showing:

    Unhandled Runtime Error
Uncaught SyntaxError: Unexpected identifier

Source
http://localhost:8080/_snowpack/pkg/-/@urql/core@v2.1.2-BaGVb5xQ5BNV3TUNPHgq/dist=es2020,mode=imports/optimized/common/5adc38f0-46a7d8df.js [:1:7]
SyntaxError: Unexpected identifier

Expected behavior

No error, successful import of @urql/core and nested dependencies.

Anything else?

From what I can tell, the problem occurs during a dependency imported from the main core.js file, when it attempts to import from "../common/5adc38f0-46a7d8df.js". I compared the results on my local devserver with those from hitting skypack directly, and found:

Skypack: https://cdn.skypack.dev/-/@urql/core@v2.1.2-BaGVb5xQ5BNV3TUNPHgq/dist=es2020,mode=imports/optimized/@urql/core.js has an import from "../common/5adc38f0-46a7d8df.js" on line 4, and https://cdn.skypack.dev/-/@urql/core@v2.1.2-BaGVb5xQ5BNV3TUNPHgq/dist=es2020,mode=imports/optimized/common/5adc38f0-46a7d8df.js resolves to a valid file.

Local DevServer: http://localhost:8080/_snowpack/pkg/-/@urql/core@v2.1.2-BaGVb5xQ5BNV3TUNPHgq/dist=es2020,mode=imports/optimized/@urql/core.js has the same import from "../common/5adc38f0-46a7d8df.js" on line 4, but the resulting URL, http://localhost:8080/_snowpack/pkg/-/@urql/core@v2.1.2-BaGVb5xQ5BNV3TUNPHgq/dist=es2020,mode=imports/optimized/common/5adc38f0-46a7d8df.js, results in the response:

Could not find resource exports/common@5adc38f0-46a7d8df.js

rather than the expected javascript content that skypack provides.

JaredReisinger commented 3 years ago

Did a little more digging... the skypack cacache (in ~/.cache/skypack) does not seem to include the expected file. Grep'ing for common/5ad recursively in the cache index directory finds the entry for @urql/core.js that includes ../common/5adc38f0-46a7d8df.js as an "X-Imports" value, but there is no file indexed whose key includes common/5ad. This would seem to explain the "Could not find resource" message, as it's simply not there!

I'm trying to get more diagnostics from snowpack devserver about its requests to skypack, but haven't managed to unearth those yet. The --verbose flag doesn't show them, and I haven't yet been able to patch any logging messages into the code that actually performs the fetch.

JaredReisinger commented 3 years ago

Okay... so it looks like this may be a skypack issue? I patched in some logging after the fetch call (https://github.com/snowpackjs/snowpack/blob/main/snowpack/src/sources/remote.ts#L115), to log the first 80 characters of the body, and saw:

GOT -/@urql/core@v2.1.2-BaGVb5xQ5BNV3TUNPHgq/dist=es2020,mode=imports/optimized/common/5adc38f0-46a7d8df.js: Could not find resource exports/common@5adc38f0-46a7d8df.js
                                                                                                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

... so it looks like the snowpack dev server is accurately passing along the data it got from skypack.

Ah... HA! Found it!

https://github.com/snowpackjs/snowpack/blob/main/snowpack/src/util.ts#L34 has:

export const remotePackageSDK = new SkypackSDK({origin: 'https://pkg.snowpack.dev'});

... but I think the skypack URL needs to be https://cdn.skypack.dev. When I patch that change in locally, at any rate, everything seems to start working. (It's also a hard-coded string, and doesn't seem to be affected by config, unless I'm missing something. I'll do some more digging, but this seems like the culprit.)

JaredReisinger commented 3 years ago

This is a very interesting rabbit-hole!

So... pkg.snowpack.dev seems to redirect to the equivalent page on cdn.skypack.dev most of the time. However... in this particular instance the evaluation of ../common/5adc38f0-46a7d8df.js results in an inconsistency depending on which host is at the root of the URL. Compare in any browser:

The second one should redirect to the first, but it actually redirects to https://cdn.skypack.dev/-/@urql/core@v2.1.2-BaGVb5xQ5BNV3TUNPHgq/dist=es2020,mode=imports/optimized/common@5adc38f0-46a7d8df.js (chaging "common / 5ad..." into "common @ 5ad..."), and that is the source of the problem.

Independent from this, it's not clear as to whether the "canonical" remote streaming module service should be cdn.skypack.dev or pkg.snowpack.dev. My sense from reading the PRs and code is that cdn.skypack.dev is the "actual" site, and the one that the 'skypack' module uses (which makes sense), while pkg.snowpack.dev is a facade so that snowpack proper can pretend to be a bit isolated from the actual site? I'm not convinced of the value of this, though, given that https://github.com/snowpackjs/snowpack/blob/main/snowpack/src/util.ts#L34 is passing this facade host into the SkypackSDK() constructor, which is imported from 'skypack', so it feels a bit silly to me to pretend it's going somewhere other than skypack. Rather than provide a fake indirection, this like could simply omit the origin option and allow SkypackSDK to use its internal default. At the very least, this would prevent snowpack proper from any any origin hard-coded.

I'm planning to do a bit more investigation, to see if:

  1. I can find where pkg.snowpack.dev is replacing the / with @
  2. it is possible to remove the completely-hardcoded pkg.snowpack.dev in snowpack/src/util.ts so that it's user-configurable with a default that falls back to the skypack internal constant

For (2), I know with 100% certainty that I can simply remove the hard-coded value, but it feels to me like the remote URL should be a user-configurable value, or at least that was the original intent. If I can figure out how to do that without making a mess of the source, I will. 😄