-
Notifications
You must be signed in to change notification settings - Fork 302
Implement Scratch.external - extensions part #2267
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
d4576f5 to
17348c0
Compare
|
Using import() is not sufficient for things like https://raw.githubusercontent.com/PsychoGoldfishNG/NewgroundsIO-JS/refs/heads/main/dist/NewgroundsIO.min.js that aren't ES modules So, we could have several different ways to import things, and make them available as Scratch.import(), Scratch.import.asPlainScript(), Scratch.import.executeInIIFEAndExport(), etc. |
I prefer letting extension developers decide whether to use |
This comment was marked as resolved.
This comment was marked as resolved.
|
no, it uses URLs to websites see #2264's imports for general kind of syntax that might work, though actually that PR and this one don't integrate quite right yet, and there's still some missing mechanisms that need to be added there's a lot in the air - might restrict it to certain websites only (trusted, unlikely to vanish, operated by neutral parties that won't do anything weird with traffic logs) or make it skip the CDNs and download from npm instead. lots of things like that still being decided there's some nice properties of this general approach:
|
This comment was marked as resolved.
This comment was marked as resolved.
|
If your school blocks major CDNs then you have much larger problems, but also the point of this PR is that all the dependencies will get inlined during a production build which solves that problem |
|
The URLs in the source code get used in development (easier to debug) while in production they get transparently converted into another format |
|
Re: bundlers were mentioned earlier I don't intend to bring in bundlers right now. The goal is to use the files provided by upstream without modification. That's needed for non-production usage where the URLs are left unchanged, is a lot simpler, and I think most of the dependencies that people want to integrate tend to offer scripts that work without changes. Biggest hurdle is that few dependencies offer ES modules. Hence the idea of allowing an import-as-script-tag or something that fetches the source code and evals it in an IIFE Letting people configure bundlers inevitably leads to dependencies requiring random versions of rollup or parcel or webpack or whatever with random configuration and plugins. I don't want to be maintaining that if I don't need to. If upstream doesn't provide workable files then we can figure something out on a case-by-case basis, possibly by forking a version and publishing under @turbowarp/* |
I'm not worried about me and I'm not on a school network. I'm concerned for our users who might have a whitelisted school network, but I suppose I misunderstood the fact that the inlining happens when the extension is built, not every time it's loaded, which entirely resolves that Now that I really understand what's happening here I'm realizing how much better it actually is than I thought Sorry for the confusion |
17348c0 to
db35f4a
Compare
this is the best way when possible since it gets inlined as literal code in production build
|
Initially, face sensing became 16MB larger with this. Cut that about in half by not supporting devices without WASM SIMD. Cut in half again by putting the WASM inside a zip and inflating it at runtime along with using base85 instead of base64. Now it's down to 3.1MB. Still a bit excessive -- wouldn't have merged this if Scratch didn't add it first -- but tolerable. |
|
It'll come down to 2.7MB by switching to fzstd instead of relying on jszip. It'll also load faster that way |
|
Down to 2.2MB by compressing the tensorflow scripts. Might end up reverting that later since it does make things take a fair bit longer to start up |
5 new APIs:
In development, all of these call directly into VM implementation which does actually go and fetch each time the extension runs. In production, all of these are inlined in various ways so that everything is inside the JS file. All references to Scratch.external go away, so we're not breaking compatibility with anything. A new concept called "build snippets" are added into built JS as needed to implement things like base85 and zstd decompression.
URLs should point to jsdelivr, unpkg, raw.githubusercontent.com, etc. In development mode, new URLs will be found and fetched so their SHA-256 can be stored. In production builds, these resources get fetched and the hashes are checked to ensure no tampering. Must use an immutable URL pinned to an npm tag or commit hash.
Face sensing has been updated to use these as a first example.
Ideally this API would be called Scratch.import, but using "import" verbatim causes various issues in TypeScript and elsewhere so we're avoiding that. Hence, "Scratch.external" and "importModule"