Skip to content
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

Open
KSXGitHub opened this issue May 31, 2020 · 27 comments
Open

Caching and loading non-importable resources #5987

KSXGitHub opened this issue May 31, 2020 · 27 comments
Labels
cli related to cli/ dir suggestion suggestions for new features (yet to be agreed)

Comments

@KSXGitHub
Copy link
Contributor

KSXGitHub commented May 31, 2020

Problem

I often find myself needing to load non-JavaScript non-TypeScript assets, and various problems arise:

  • Local module (in git repo) has to use fs but remote module has to use fetch.
  • The assets cannot be cached.
  • The assets cannot be bundled.

Suggestion

Add some way to annotate assets using comments, and fetch should support those assets. For example:

// @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))
// --allow-net is not required since it reads from cache

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 and bytes are being discussed in whatwg/html#9444

@nayeemrmn
Copy link
Collaborator

nayeemrmn commented May 31, 2020

Resolved

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.

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.

@KSXGitHub
Copy link
Contributor Author

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.

@ry
Copy link
Member

ry commented Jun 1, 2020

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.

@KSXGitHub
Copy link
Contributor Author

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)?

@ry
Copy link
Member

ry commented Jun 1, 2020

@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.

@nayeemrmn
Copy link
Collaborator

Plugins, too. I think the point is being able to statically depend on arbitrary non-importable resources, not discussing what resources can be imported.

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...

  • The Web provides no way for scripts to do this -- statically. They have to fetch() them at runtime. The same is currently true for Deno.
  • On the web you can pre-fetch resources in the HTML context:
    <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.
  • Normally I would say "Why? Modules are for code-splitting. Modules shouldn't be managing resources in a local way." but the practical uses are too many.
  • The reason Deno might especially need this is because those runtime fetches require --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.

@KSXGitHub KSXGitHub changed the title Caching and loading non-JavaScript non-TypeScript assets Caching and loading non-importable resources Jun 1, 2020
@KSXGitHub
Copy link
Contributor Author

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"?

@kitsonk
Copy link
Contributor

kitsonk commented Jun 2, 2020

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.

@KSXGitHub
Copy link
Contributor Author

Making fetch() cacheable

This is what this issue is based on. The difference is that Deno should preload resources even before executing code.

@bartlomieju bartlomieju added cli related to cli/ dir feat new feature (which has been agreed to/accepted) labels Jun 7, 2020
@guybedford
Copy link
Contributor

@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?

@kitsonk
Copy link
Contributor

kitsonk commented Dec 17, 2020

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 imports or other mechanisms of loading assets?

@kitsonk kitsonk added suggestion suggestions for new features (yet to be agreed) and removed feat new feature (which has been agreed to/accepted) labels Dec 17, 2020
@guybedford
Copy link
Contributor

guybedford commented Dec 17, 2020

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.

@KSXGitHub
Copy link
Contributor Author

KSXGitHub commented Dec 17, 2020

@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.

@guybedford
Copy link
Contributor

guybedford commented Dec 17, 2020 via email

@KSXGitHub
Copy link
Contributor Author

KSXGitHub commented Dec 17, 2020

@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'))

@guybedford
Copy link
Contributor

@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.

@KSXGitHub
Copy link
Contributor Author

KSXGitHub commented Dec 18, 2020

@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.

@guybedford
Copy link
Contributor

guybedford commented Dec 18, 2020 via email

@shivajivarma
Copy link

Any update on this? How to ship static assets like styles, scripts, html from within module.

@KSXGitHub
Copy link
Contributor Author

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.

@KSXGitHub
Copy link
Contributor Author

Actually, import assertion doesn't cover generic file types (such as text files, binary blobs, etc) so forget it.

@kt3k
Copy link
Member

kt3k commented Nov 26, 2024

deno compile now supports --include flag which embeds arbitrary assets to the built binary. #26934

Can this issue be considered resolved now?

@inverted-capital
Copy link

deno compile now supports --include flag which embeds arbitrary assets to the built binary. #26934

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

@kt3k
Copy link
Member

kt3k commented Nov 26, 2024

In deno deploy, you can have access to files in your linked git repository https://docs.deno.com/deploy/api/runtime-fs/

@KSXGitHub
Copy link
Contributor Author

KSXGitHub commented Nov 26, 2024

@kt3k The ideal solution should have the following properties:

  • It should work seamlessly with 3rd party libraries. Dependencies should be able to declare non-importable assets.
  • It should work the same regardless of environment: independent remote file, independent local file, bundle, deploy, etc.

@petamoriken
Copy link
Contributor

For reference, text modules and bytes modules are under discussion at web standards.
whatwg/html#9444

@inverted-capital
Copy link

In deno deploy, you can have access to files in your linked git repository https://docs.deno.com/deploy/api/runtime-fs/

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to cli/ dir suggestion suggestions for new features (yet to be agreed)
Projects
None yet
Development

No branches or pull requests

10 participants