| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
wasm is already a "module". i don't think this change is quite right. |
Sorry, something went wrong.
|
@devsnek package types are format mappings, this just adds another set of format mappings to allow files without extensions to be seen as WASM. |
Sorry, something went wrong.
interesting. i guess we should change the logic to just pass whatever value is present straight through to the resolver unless it equals "commonjs" right? (and maybe rename "module") |
Sorry, something went wrong.
|
@devsnek I think that passing things around to the resolver would be a different API/PR. People have already asked about a getPackageType function in other issues and that might be simpler. |
Sorry, something went wrong.
|
What I dislike about this change is that type previously had a relatively simple meaning: It determined the file format of both .js files and files without extension:
After this lands, there's now a gray area: wasm means "interpret no extension as WASM" but .js is left ambiguous. Without reading too closely I assume it interprets .js as... a JS module? But it's certainly not obvious to me. Maybe an object syntax would be more helpful at that point? {
"type": {
"ambiguous javascript": ": "module",
"extension-less": "wasm"
}
}But that opens a whole can of worms about configuring all kinds of file extension mappings. I'm pretty close to saying "do we need to support extension-less wasm"? |
Sorry, something went wrong.
|
This makes me again question the package.json field additions :( |
Sorry, something went wrong.
|
@jkrems how is ".js" ambiguous? It has a specified value (which is "module" in this case as you guessed)
The same could have been said about extension-less ESM. I'm not sure how wanting to run a purely WASM application would be considered a bad thing given WASI and the like coming in the future. |
Sorry, something went wrong.
|
I'm not opposed to support for running a purely WASM application. But the field name is awkward to me. Right now it can be read as "(content) type of inherently ambiguous files like .js and files lacking an extension". With this change, it's "(content) type of extension-less files and sometimes also of .js, you have to know from context if that applies". I think I would even prefer if this PR would treat .js within those packages as WASM. That would be consistent with what the field currently does. It wouldn't break people who want to ship a purely WASM app and everyone else can use .mjs. |
Sorry, something went wrong.
|
We could also just not give .js a format, and then it would error. .mjs and
.cjs would still work. Seems better than forcing a new extension onto WASM
and we could then ask the MIME owner about adding .js if truly desired for
this pr to land.
…On Thu, Jan 16, 2020, 5:19 PM Jan Olaf Krems ***@***.***> wrote:
I'm not opposed to support for running a purely WASM application. But the
field name is awkward to me. Right now it can be read as "(content) type
of inherently ambiguous files like .js and files lacking an extension".
With this change, it's "(content) type of extension-less files and
sometimes also of .js, you have to know from context if that applies".
I think I would even prefer if this PR would treat .js within those
packages as WASM. That would be consistent with what the field currently
does. That wouldn't break people who want to ship a purely WASM app and
everyone else can use .mjs.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#31388?email_source=notifications&email_token=AABZJI4BQSR4RVOO3A7K7YLQ6DTRFA5CNFSM4KH2TNG2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJF4YXY#issuecomment-575392863>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABZJI3LS4NSYQV6A2YMEMLQ6DTRFANCNFSM4KH2TNGQ>
.
|
Sorry, something went wrong.
|
Throwing on .js in that mode also seems fine. That means we can effectively still say it's the field that determines the type for all ambiguous files since few people will try to put WASM into .js in practice. |
Sorry, something went wrong.
|
(See nodejs/modules#283 for a prediction of this inevitable occurrence) |
Sorry, something went wrong.
I do not want the focus of the PR to be on preserving this idea. Already this is not true (see .node in "commonjs") and other features toggle in the presence of type like if to use the ESM loader for the main module. If that is a invariant we wish to make we seem to have already failed and would probably need the loader to be reverted to have that invariant in other PRs. My desire is for the focus of this PR is solely on ensuring we have a way for WASM to be supported at the same level as other first class sources by enabling loading WASM from files without extensions. |
Sorry, something went wrong.
|
I don’t see the purpose of this. If WASM files are always parse goal Module, then they’re unaffected by the type field; and therefore we don’t need any new values for type. The .wasm extension is equivalent to .mjs or .cjs in that regard. |
Sorry, something went wrong.
Can you clarify? WASM files can be entry points of a program and there it can be nice if we don't force the user experience to be ./app.wasm (only ever works with extension) instead of ./app (WASM files without extension supported). |
Sorry, something went wrong.
You’re talking about from the command line, e.g. node ./app? No, that doesn’t seem worth the confusion and added complexity around "type". I think it’s simpler to just say that Node always treats extensionless files as JavaScript, equivalent to if it were a .js file (so parse goal defined by controlling "type"). If they want node ./app or just ./app, they should create an app extensionless JavaScript file that contains import('./app.wasm'). |
Sorry, something went wrong.
|
that there would be a difference is a signal that something is wrong with our resolver. it should behave consistently no matter the placement of a specifier. |
Sorry, something went wrong.
You could've said the same thing for ESM: just create an extension-less CommonJS file and import() the ESM entry point. Purely WASM apps are likely to be a thing in the future. Forcing a compiler that generates otherwise self-contained WASM binaries to always write a one-line JS files to start the binary doesn't feel realistic. WASM is a first-class source type, so it has the right to exist as extension-less files imo. |
Sorry, something went wrong.
|
I would also like to point out that WASM vs JS is seen as a security concern by some and is part of the discussion of https://github.com/tc39/proposal-module-attributes , to people of that mindset it might be undesirable to have any JS in their application. |
Sorry, something went wrong.
This is really asking for an entirely new feature. Currently all extensionless files are treated as JavaScript, and type controls how they’re parsed (as CommonJS or as ESM). We don’t have a way to define as what file type extensionless files are interpreted, e.g. as JavaScript versus as native modules versus as WASM. That would probably entail a new package.json field, something like "extensionlessTreatedAs": ".js" (the current/default behavior). I think there’s certainly no rush to add such a feature. I’ve never heard discussed that Node would treat anything other than JavaScript as equivalent first-class with JavaScript, and that’s a broader discussion that should be had before we start doing so. Having a one-line import './app.wasm' extensionless JavaScript file feels good enough for now, at least. |
Sorry, something went wrong.
|
one of the main design goals of esm is to allow loading resources of any type. if we can load a format, we should be able to load it from any position that source text modules can be loaded from. if it can't be, it's an artificial limitation we should correct. |
Sorry, something went wrong.
That feels like a very blurry definition of a "file type". CommonJS and ES modules are two different file types. They both happen to be text files while WASM is a binary format. But just because both are text (and their syntax has some overlap) doesn't mean they are the same file type. I can also write some CoffeeScript that parses as CommonJS and/or ES modules. Doesn't mean that CoffeeScript isn't a different file type. :) |
Sorry, something went wrong.
Why is this important? Using "type": "wasm" means that a user would also need a package.json file, so what’s the use case where the user can make a folder with at least two files (the WASM file and the pacakge.json) but can’t (or wouldn’t want to) also put an extension on the WASM file? Having "type": "module" support extensionless files predates "exports"; it was intended to permit pretty specifiers, e.g. import shuffle from 'lodash/shuffle' where shuffle was an extensionless ES module file. Now that we have "exports", I don’t see why "type" should affect extensionless files. I especially don’t think that "type" should affect extensionless files if that means that we will need new values for "type" for every new type of entry point that Node ever supports. We should probably just drop support for extensionless files in the ESM loader while it’s still experimental. The CommonJS loader will always load such files as CommonJS JavaScript. |
Sorry, something went wrong.
|
I'm curious about how could ./app work with a WASM file. Since it's a binary format, it cannot contain a shebang, or can it? |
Sorry, something went wrong.
|
@targos you can create a binfmt entry: :wasm:M::\x00\x61\x73\x6d::/path/to/node: |
Sorry, something went wrong.
|
#31415 is removing support for any extension-less files and supersedes this for now. A different PR can be opened if other features are found to warrant this feature. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Checklist
The main benefit of this is to allow files without an extension to be treated as WASM. This does alter the logic for detecting if something should be treated as going through the ESM loader for the main module slightly by inverting the condition from checking for "module" to instead check it doesn't match "commonjs"
This remains flagged behind the WASM modules flag