Skip to content
This repository has been archived by the owner on Dec 16, 2021. It is now read-only.

Fun with svelte/foo resolution #22

Closed
Conduitry opened this issue May 30, 2019 · 7 comments · Fixed by #27 or #28
Closed

Fun with svelte/foo resolution #22

Conduitry opened this issue May 30, 2019 · 7 comments · Fixed by #27 or #28
Labels
bug Something isn't working

Comments

@Conduitry
Copy link
Member

The current version of the REPL resolves svelte/foo to unpkg.com/svelte/foo.mjs. This will break if a new version of Svelte is released in its current form, because svelte/foo ought to resolve to svelte/foo/index.mjs.

The new version also includes svelte/foo/package.json which should theoretically make unpkg.com/svelte/foo?module resolve directly to unpkg.com/svelte/foo/index.mjs via unpkg. But this doesn't work in the existing versions of Svelte, because unpkg.com/foo?module will just give us the CJS version.

The most obvious solution is ugly: inspect the version number, and if it's less than or equal to 3.4.4, go with the old logic unpkg.com/svelte/foo.mjs, and if it's greater, go with unpkg.com/svelte/foo?module.

Also potentially an issue: https://unpkg.com/svelte@3.4.4/index.mjs?module right now contains an ESM module (as expected, since unpkg respects package.json), but the import is from ./internal?module. I don't know whether this is going to be a problem. This would happen with all of the requests and responses if we went forward with the changes outlined here.

@Conduitry Conduitry added the bug Something isn't working label May 30, 2019
@Conduitry
Copy link
Member Author

A good idea from @mrkishi - we could request https://unpkg.com/svelte/?meta and get a list of everything in the package and use that to help figure out what should resolve to what.

@Conduitry
Copy link
Member Author

I just realized, this is probably complicated by this recent addition. I don't know what the particular goal of that feature was, but it's probably incorrect to assume we'll get the same features out of whatever offline package registry as we can get from unpkg.

@Conduitry
Copy link
Member Author

It wouldn't be the easiest, but it would probably make the most sense to make the whole resolution part of this pluggable, a la Real Rollup. I don't know how else we're going to support proper resolution and yet also allow things to be loaded from local files.

@Conduitry
Copy link
Member Author

Ugh nope not quite resolved with that PR. #27 assumes that svelteUrl is going to end with @3.x.x and that it can just look at that to decide what version this is. But that is not quite the case. The REPL is initially loaded with @3 (which then redirects to the latest matching version), and so when the REPL initially sees the svelteUrl, it's not @3.x.x and so it uses the new filenames, which will break currently as 3.4.4 is still the latest version and it has the old filenames. God damn it this is so annoying.

@mrkishi
Copy link
Member

mrkishi commented Jun 3, 2019

Does init get called multiple times?

I assume you could use svelte.VERSION there instead — or was this added later in Svelte's life?

@mrkishi
Copy link
Member

mrkishi commented Jun 3, 2019

or was this added later in Svelte's life?

Post-v1, at v1.42.

@Conduitry
Copy link
Member Author

I think using svelte.VERSION might work. We don't need to worry about versions of Svelte before 3, and I'm pretty sure Svelte will already be loaded at this point, because importScripts() is synchronous.

We can also simplify some other stuff because it's a lot safer to depend on svelte.VERSION having a particular format than the URL having one. This will mean that version=local will temporarily fail (since svelte.VERSION is still 3.4.4 but the package layout has already changed on master) but I'm not going to worry about that.

New PR incoming.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
2 participants