-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Caching and loading non-importable resources #5987
Comments
ResolvedI don't think // deno-asset "./assets/foo.wasm"
// deno-asset "./assets/bar.png"
await fetch(new URL("assets/foo.wasm", import.meta.url));
await fetch(new URL("assets/bar.png", import.meta.url));
// The above skip net permission because they fetch cached, statically dependent
// resources -- similar to using imports from a static code dependency.
// As a special case they should work for `file://` URLs so both local and
// remote modules/resources will work. Ref #4001. Nevertheless, the problem of "modules needing to statically depend on non-code resources" exists in web browsers. Even though the proposal you linked is most likely going nowhere, it's still risky for Deno to invent its own solution. |
This is simpler, let's use it.
I suppose bundlers have that problem too? Maybe we could use some references.
We can hide it behind |
I think we'll add We had this feature before, but removed it before 1.0 because we felt it had not seen enough usage to commit it to it. |
This is good to hear. What about other kinds of resources (such as images)? |
@KSXGitHub I'm quite open to adding support for JSON - we also removed that before 1.0 due to some security concerns. I don't know about images - we need to investigate what's happening in the web standards proposals. |
Plugins, too. I think the point is being able to statically depend on arbitrary non-
I've thought more about the Web vs Deno on this issue...
|
I think we can use this (in triple-slash comment) instead of /// <link rel='preload' href='./assets/foo.wasm' as='fetch' />
/// <link rel='preload' href='./assets/bar.png' as='fetch' />
await fetch(new URL('./assets/foo.wasm', import.meta.url))
await fetch(new URL('./assets/bar.png', import.meta.url))
// --allow-net is not required since it reads from cache Does this still count as "Deno inventing its own solution"? |
My opinion. We should only allow importing resources that logically represent code which Deno can execute and have a statically determinable binding surface. Deno can execute Web Assembly and there are browser standards in flight to talk about how it exposes itself in JavaScript. It can execute plugins and Deno's own APIs determine the bindings. Images are not code that Deno can execute. CSS is not code that Deno can execute. JSON can be interpreted as code, and there are established conventions on how to determine binding patterns, though it isn't part of current standards tracks. Making |
This is what this issue is based on. The difference is that Deno should preload resources even before executing code. |
@kitsonk in ncc, we detect and emit asset references when bundling, relocating exactly references like a |
I still don't know... I almost don't like anything that isn't importable, and I personally would rather build on import assertions, and address bundling (as well as potentially dealing with registrations of loaders for assertions as well), as the way of bringing in assets. We already have challenges with dynamic @guybedford when you say ncc detects asset references, are you talking about arbitrary |
The goal of ncc, like Deno bundle is to enable single file builds of many modules where most of the code is third party code. Local file imports are a fact of life for real world libraries. Protobufs, Wasm files, native modules (Node.js only, not necessarily a Deno concern yet), etc. Webpack let us import everything and that lets us use the resolver so that's sort of the pattern, but Node.js imports (and browser imports) are much more limited and don't allow arbitrary asset imports. Therefore an "asset emission" is always necessary for bundling Node.js code, and I think the same pattern would be useful in Deno. In ncc, we explicitly detect very complex execution analysis like Asset expressions are detected using any contextual file name reference ( This RollupJS plugin does the same thing for ES modules using the So for ES modules I think the story with this expression provides a very useful pattern and technique that can support everything from protobuf loading to Web Assembly in browsers, Node.js and Deno which makes it very versatile. Getting awareness to bundlers is the first step though as right now it's just in plugin systems like the one above. |
@kitsonk Loaders don't work in the browser. As for import assertion, it seems cool, does it allow defining arbitrary mime types or simply throw an error when assertion and actual content type do not match? @guybedford This approach seems error-prone. While it should work in most cases, there are few cases where it causes subtle difference in behavior between unbundled and bundled files. |
Can you give an example of cases which break down here?
…On Wed, Dec 16, 2020 at 22:03 Khải ***@***.***> wrote:
@kitsonk <https://github.com/kitsonk> Loaders don't work in the browser.
As for import assertion, it seems cool, does it allow defining arbitrary
mime types or simply throw an error when assertion and actual content type
does not match?
@guybedford <https://github.com/guybedford> This approach seems
error-prone. While it should work in most cases, there are few cases where
it causes subtle difference in behavior between unbundled and bundled files.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5987 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAESFSSFVCDUC5POMKKLPRDSVGNLLANCNFSM4NO6MWYA>
.
|
Can ncc or any bundler handle this? // asset/mod.ts
export function asset(name: string): URL {
return new URL('../assets/' + name, import.meta.url)
} // consumer/mod.ts
import { asset } from '../asset/mod.ts'
await fetch(asset('foo.png'))
await fetch(asset('bar.png')) |
@KSXGitHub firstly note that ncc doesn't support So in ncc, the equivalent of an expression like Note that interestingly all the same logic running above is exactly the same as the logic needed to handle dynamic import expression emission (well except they emit new entry points not assets in the bundler). So the idea that modules are "cleaner" in this problem space is easily dismissed based on this too. |
@guybedford One of the reasons for this issue was the inconsistency between loading assets in a local module and loading assets in a remote module (a local module has to read from the filesystem while a remote module must use |
Many Deno users including myself hook fetch to support file URLs, and yes
thats how i found this issue. I just pointed out these arguments as the
validity of these patterns seemed like a hesitation here when in fact I
think they are very important ESM patterns.
…On Thu, Dec 17, 2020 at 18:28 Khải ***@***.***> wrote:
@guybedford <https://github.com/guybedford> One of the reason for this
issue was the inconsistency between loading assets in a local module and
loading assets in a remote module (local module has to read from filesystem
while remote module must use fetch()). Your proposal would only solve
this problem for bundled code.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5987 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAESFSTZM2BUHPCDEQHZ7C3SVK46RANCNFSM4NO6MWYA>
.
|
Any update on this? How to ship static assets like styles, scripts, html from within module. |
Import assertion has been merged by TypeScript: microsoft/TypeScript#40698. I think that all that is left is waiting for Deno developers to implement caching mechanism. |
Actually, import assertion doesn't cover generic file types (such as text files, binary blobs, etc) so forget it. |
Can this issue be considered resolved now? |
I don't think it can, since we cannot import assets outside of compile, like in deno deploy, for example |
In deno deploy, you can have access to files in your linked git repository https://docs.deno.com/deploy/api/runtime-fs/ |
@kt3k The ideal solution should have the following properties:
|
For reference, text modules and bytes modules are under discussion at web standards. |
Yes, but I cannot have access to the file in the packages that I depend upon. For example if I need to import a text prompt from a package like https://jsr.io/@artifact/dumb-bot/0.0.15/instructions.md then I can't do that using the import system. The import system would take care of caching, integrity, and synchronous access. But without it I have to choose one of several paths to read that file in myself and I'll probably mess it up somewhere, or have to explain it to my colleagues. Yes there are workarounds for all this, I think we in this thread are simply campaigning for it to be part of the base solution that Deno supplies, since it feels like the files in question are so close we can almost touch them, but can't. |
Problem
I often find myself needing to load non-JavaScript non-TypeScript assets, and various problems arise:
fs
but remote module has to usefetch
.Suggestion
Add some way to annotate assets using comments, and
fetch
should support those assets. For example:Alternatives
There is asset reference proposal that is likely not going to get to stage 3 anytime soon.import assertion proposal had reached stage 3.
Import types
text
andbytes
are being discussed in whatwg/html#9444The text was updated successfully, but these errors were encountered: