feat: allow version to be a pr-[prnumber] or commit-[commithash]

[paoloricciuti]

So...this basically allow us to open the playground with a version which is a pkg.pr.new url (generated from our CI on PRs and pushes) and it will use that version of svelte.

This is working but i think there's some thing to discuss about the implementation and i wanted some feedback before (p.s. i also paired on it with @oscard0m this evening).

EDIT: i actually went ahead and implemented the solution below...so now to access a specific PR or commit you would do https://svelte.dev/playground/hello-world?version=ref:13953...i've also removed all the is_pkg_pr_new around...we can always revert that commit!

1. currently you should access the playground like this https://svelte.dev/playground/hello-world?version=https://pkg.pr.new/svelte@13953 to use that version of svelte... @Conduitry was arguing that maybe we shouldn't require the whole URL so that we can detach ourselves from pkg.pr.new in the future. I think it's a fair point so maybe we can change this.
2. if we stick with the whole url i made the decision to check for the specific pkg.pr.new URL to prevent people from putting urls in query param and make us fetch their url.
3. to untar the package i've used this library here...it has been published something like 3 weeks ago, very early version but the code is not that complex and it seems to be doing everything alright (and it works)...it doesn't have types hence the couple of .d.ts that i manually generated. In theory we could remove the dependency and just copy paste the whole file in the codebase without much hassle.
4. I'm also passing around a lot of is_pkg_pr_new variables which i don't particularly like.
5. Since i already fetch all the files from the tarball i'm using those in the rollup plugin (no need to fetch from unpkg if we already have the files)

How i think we can make this better.

1. we could instead of using the whole url do something like ref:13953 as the version (i'm using ref because technically you could also use a commit hash). This will make sure that if tomorrow we want to leave pkg.pr.new we could find a different way to fetch the code for that pr/commit instead.
2. we could use a different package to untar stuff? I didn't found one that work (it actually needs to handle tar.gz but if you have suggestions for a more stable package i can swap it out...we can also just copy the code.
3. instead of passing around is_pkg_pr_new everywhere we could infer it either from the version (is we change to ref:[number] we could just check if adhere to this format) or from the url itself (if we stick to pkg.pr.new i can still check that the version is an URL with that origin.

Example:

using this link

https://svelte-dev-git-pkg-pr-new-version-svelte.vercel.app/playground/hello-world?version=pr-13953

you can access the test of the pr 13953 (i picked that pr because it's very easy to verify that it's actually using that updated compiler)

WDYT?

Also i tried to stress test the playground a bit and it seems to work fine but i hope i'm not missing anything 😄

Before submitting the PR, please make sure you do the following

• It's really useful if your PR references an issue where it is discussed ahead of time.
• Prefix your PR title with feat:, fix:, chore:, or docs:.
• This message body should clearly illustrate what problems it solves.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
svelte-dev	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 5, 2024 3:19am

[Rich-Harris]

This is cool! I opened a PR on the tarparser repo to add types there, so that every user of the library doesn't have to duplicate the declare module effort: highercomve/tarparser#1

I agree with @Conduitry that it should be pr-nnnnn rather than a URL:

• it's more future-proof
• it doesn't suggest to users that they can use arbitrary URLs (e.g. registry.npmjs.org or some other source)
• it's less to type
• it doesn't involve characters that need to be URI-encoded

Similarly I don't think it should be ref: — PR numbers aren't refs, and we shouldn't treat them as the same sort of thing as commit hashes or tags or branch names. We can always add support for refs in future if we find a need.

[paoloricciuti]

Similarly I don't think it should be ref: — PR numbers aren't refs, and we shouldn't treat them as the same sort of thing as commit hashes or tags or branch names. We can always add support for refs in future if we find a need.

I actually already made the change to use ref: but I can change it to pr-...but since pr.pkg.new actually create one for every commit to main should we just also already add a sha-shshsh or ref-272727 together with pr-2883?

[paoloricciuti]

Ok i've implemented both pr-83763 and commit-kjasdha...i've did that in two separate commits so that we can always revert commit if we don't like it.

[Rich-Harris]

awesome!