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

What to do about the --browser flag? #1332

Open
alexcrichton opened this issue Mar 8, 2019 · 8 comments
Open

What to do about the --browser flag? #1332

alexcrichton opened this issue Mar 8, 2019 · 8 comments

Comments

@alexcrichton
Copy link
Contributor

With RFC 6 now merged and implemented, along with a tweak or two, we now have the following output modes:

  • no flags specified - generate "bundler compatible" as well as future-compatible output where wasm files are assumed to be an ES module and everything is an ES module
  • --nodejs - generate CommonJS imports as well as assuming a synchronous instantiation is ok. Also assumes node.js for various support APIs.
  • --web - generates an ES module which exports a function to manually instantiate the wasm module. Intended for raw inclusion on web pages today.
  • --no-modules - like --web, except incompatible with js snippets and provides a global instead of an ES module.

In addition to all these flags we have this weird --browser flag sitting on the site. The only real purpose of this flag any more is to indicate that when no flags are specified (generating "bundler compatible output") the JS should assume that node.js isn't being used and browser APIs are always available. This is a slight optimization over not passing the flag as it allows us to elide the check for things like "where is TextDecoder?" and such.

The --browser flag has a pretty unfortunate name to say the least, being pretty confusing with the other flags. Should we keep the use case of slimming the JS by a few bytes? Should this just be deprecated and removed?

@fitzgen
Copy link
Member

fitzgen commented Mar 8, 2019

Can we deprecate the flag and rename it to more accurately reflect its function? --assume-web-apis-instead-of-feature-detection or somethign slightly less verbose? :-p

@Pauan
Copy link
Contributor

Pauan commented Mar 9, 2019

Yeah, replacing it with something like --modern (or similar) sounds good to me.

If it were a separate flag, then it would be useful with any of the modes, e.g. assuming modern Node versions, assuming modern browser versions, etc.

@alexcrichton
Copy link
Contributor Author

@fitzgen yeah I think that's definitely an option, and may end up what we get at.

@Pauan oh currently it's not so much about --modern but for example on Node the TextEncoder API is loaded from require('utils') whereas it's just ambiently available on the web. Now we also have a flag that controls detection of TextEncoder#encodeInto which would fit well with the idea of a --modern flag, but even the most modern node.js doesn't have TextEncoder ambiently available (I think, right?)

@LuoZijun
Copy link

@alexcrichton Node v11.10.0 have TextEncoder :)

@alexcrichton
Copy link
Contributor Author

Oh nice! I think that literally the only use case that --browser modifies then is how TextEncoder is lowered.

So actually I think I agree with @Pauan in light of that. @Pauan to make sure I understand right though, I think we could:

  • Add a new --modern flag indicating "assume the latest and greatest JS standards". Things like TextEncoder#encodeInto and the location of TextEncoder would change based on this.
  • Deprecated/remove --browser in favor of --modern
  • Deprecated/remove --encode-into in favor of --modern

I think we could even go with that --encode-into does today, and do something like --modern {test,always,never} to indicate whether we feature detect, assume it's there, or go backwards as far as we can (defaulting to --modern test)

@Pauan
Copy link
Contributor

Pauan commented Mar 12, 2019

@alexcrichton That sounds reasonable to me. I'm not sure about the value of never, but I'm not against it.

@daxpedda
Copy link
Collaborator

daxpedda commented Jan 3, 2020

We should change the file extension of modern to be mjs while we are at it.
Would it be viable to do this for web to?

@Pauan
Copy link
Contributor

Pauan commented Jan 4, 2020

@daxpedda The situation with mjs is really complicated. In particular, it has nothing to do with whether things are "modern" or not, instead it should only be used for ES6 modules.

So, outputting mjs for web and bundler would make sense, but it would be wrong to do so for nodejs or no-modules, because they are not ES6 modules.

And it's not really needed to output mjs at all, since you can set "type": "module" in your package.json, which will cause your .js files to be treated as ES6 by Node.

This will give you better compatibility with existing tools which expect js (bundlers, IDEs, web servers, analyzers, syntax highlighters, etc.)

Regardless of whether you use js or mjs, it is recommended to always set "type" to "module" or "commonjs", so that way it is unambiguous:

Package authors should include the "type" field, even in packages where all sources are CommonJS. Being explicit about the type of the package will future-proof the package in case the default type of Node.js ever changes, and it will also make things easier for build tools and loaders to determine how the files in the package should be interpreted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants