Open KSXGitHub opened 4 years ago
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.
I don't think
fetch()
has room for these special identifiers since those are valid URLs, and it wouldn't be browser compatible. Also those URLs are relative to the current module, right? Regular runtime functions can't determine or change behaviour based on the current module URL, so you need to resolve it explicitly:// 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.
This is simpler, let's use it.
Nevertheless, the problem of "modules needing to statically depend on non-code resources" exists in web browsers
I suppose bundlers have that problem too? Maybe we could use some references.
it's still risky for Deno to invent its own solution.
We can hide it behind --unstable
flag.
I think we'll add import foo from "foo.wasm"
soon.
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.
I think we'll add
import foo from "foo.wasm"
soon.
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-import
able resources, not discussing what resources can be import
ed.
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.
I've thought more about the Web vs Deno on this issue...
fetch()
them at runtime. The same is currently true for Deno.<link rel="preload" href="bar.png" as="fetch">
Deno's analogy to this would be listing some pre-fetched URLs in a command line flag... not very useful. What's requested is a way for a module to distributively declare it's own dependency on a resource as it could for an import
.
--allow-net
. Why should they? Those are statically identified resources. Ideally, they're viewed the same as code dependencies from a sandbox perspective. The proposal here uses comments as a way to download them at compile time instead.
- On the web you can pre-fetch resources in the HTML context:
<link rel="preload" href="bar.png" as="fetch">
I think we can use this (in triple-slash comment) instead of // @deno-asset
. Example:
/// <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 fetch()
cacheable and working with local files (both which have open issues) should be the primary API used for non-code assets. Potentially plugins processors for the runtime compiler APIs and the support of custom compilers (which also both have existing issues) would be ways users could extend Deno in a supportable fashion to support other resources they wish to treat as code.
Making
fetch()
cacheable
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 new URL('./x', import.meta.url)
and copying them into the output folder. This is a pattern I would like to see all JS build tools supporting in future and I think Deno bundle should as well. As such, perhaps that changes the arguments here somewhat?
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 import()
as well (as well as workers), where we need to resolve and enhance those for deno compile
/deno bundle
as well.
@guybedford when you say ncc detects asset references, are you talking about arbitrary require()
s and import
s or other mechanisms of loading assets?
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 const file = path.resolve(__dirname, 'asset.txt); fs.readFile(file)
being mapped into fs.readFile(assetBase + './asset-9f8g7d9f.txt')
where the build process "emits and hashes" assets.
Asset expressions are detected using any contextual file name reference (process.cwd()
, path.resolve()
, __filename
and __dirname
) that evaluates to an exact file that is referenced from the library that clearly will no longer be referenceable once bundled.
This RollupJS plugin does the same thing for ES modules using the new URL('./asset', import.meta.url')
emission which is much simpler than the ncc analysis - https://modern-web.dev/docs/building/rollup-plugin-import-meta-assets/.
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 notifications@github.com 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 https://github.com/denoland/deno/issues/5987#issuecomment-747228428, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAESFSSFVCDUC5POMKKLPRDSVGNLLANCNFSM4NO6MWYA .
@guybedford
Can you give an example of cases which break down here?
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 new URL('./x', import.meta.url)
currently at all because this pattern isn't in wide enough use for that work to have been necessary yet, but we support all the analogs in CommonJS of this pattern.
So in ncc, the equivalent of an expression like new URL('../assets/' + name, import.meta.url)
is probably path.resolve(__filename, '../assets/' + name)
or something like that, which is always treated as a globbing wildcard emission based on pkgPath/assets/**
. We add extra filtering to ensure that we never emit folders outside of the package base itself so there's a sort of scoping that applies.
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 fetch()
). Your proposal would only solve this problem for bundled code.
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 notifications@github.com 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 https://github.com/denoland/deno/issues/5987#issuecomment-747827018, 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: https://github.com/microsoft/TypeScript/pull/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.
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.