-
Notifications
You must be signed in to change notification settings - Fork 322
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
Clarify recommendations regarding .mjs #228
Conversation
Resolves google/WebFundamentals#7623. The `.mjs` extension is a perfectly valid recommendation for development, but users should not be encouraged to publish JavaScript files with an `.mjs` extension unless they’re sure that such files will be served with the proper MIME type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This patch removes or rephrases several sections that AFAIK are accurate as-is. Can you help me understand the reasons behind this?
@@ -42,7 +42,7 @@ Here’s how to statically import and use the `./utils.mjs` module: | |||
``` | |||
|
|||
:::note | |||
**Note:** The previous example uses the `.mjs` extension to signal that it’s a module rather than a regular script. On the web, file extensions don’t really matter, as long as the files are served with the correct MIME type (e.g. `text/javascript` for JavaScript files) in the `Content-Type` HTTP header. The `.mjs` extension is [especially useful](https://github.com/nodejs/node-eps/blob/master/002-es-modules.md#32-determining-if-source-is-an-es-module) on other platforms such as Node.js, where there’s no concept of MIME types or other mandatory hooks such as `type="module"` to determine whether something is a module or a regular script. We’re using the same extension here for consistency across platforms and to clearly make the distinction between modules and regular scripts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like we should just keep this, and update the link to https://nodejs.org/api/esm.html#esm_enabling. We could also point out that d8
recognizes .mjs
, in case we want to add more examples than just Node.js.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ve updated my revision per your note.
The “no concept of MIME types or other mandatory hooks such as type="module"
” language isn’t quite correct. As you know, Node has "type": "module"
now, which in fairness didn’t exist back when this sentence was written; but before you argue that the current text is still correct because of the “mandatory” qualifier that was added in response to Node’s "type": "module"
, keep in mind that HTML’s type="module"
isn’t mandatory either: a regular script can load a module via import()
, with no type="module"
in sight.
But that’s just one clause, and the paragraph is stronger without it. .mjs
’s utility is that it indicates a file’s module-ness for all non-browser environments and tools, rather than Node.js’ "type": "module"
or Spidermonkey’s -m
or the various proprietary configuration options in Babel and Webpack and so on. I think that point comes across more clearly without the digression into .mjs
’s competitors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The “no concept of MIME types or other mandatory hooks such as
type="module"
” language isn’t quite correct. As you know, Node has"type": "module"
now, which in fairness didn’t exist back when this sentence was written; but before you argue that the current text is still correct because of the “mandatory” qualifier, keep in mind that HTML’stype="module"
isn’t mandatory either: a regular script can load a module viaimport()
, with notype="module"
in sight.
Yeah, but the script doing the dynamic import()
would be a classic script, not a module. The current phrasing is precise and correct, and I am not in favor of removing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Allow me to be blunt, if you don’t mind. The original sentence was this:
The
.mjs
extension is especially useful on other platforms such as Node.js, where there’s no concept of MIME types or other hooks such astype="module"
to determine whether something is a module or a regular script.
Then "type": "module"
came to Node, and so that sentence was no longer true. So you added “mandatory” before “hooks.”
As currently written, “mandatory” is a weasel word. It makes the sentence true again (arguably) but it is misleading to the reader in that it implies that .mjs
is the only way to use ESM in Node. No reasonable reader would read this sentence and assume that there was any way to use ESM in Node other than via .mjs
. No one would read this and think, “oh, but does Node have any non-mandatory hooks?”
If you want to recommend .mjs
, that’s fine; my revised text makes that case for you, without deceptive language.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P.S. to my last comment: I didn’t mean to imply that you were being deceptive in your edit. I only meant that it comes across that way to me, and I’m hardly unbiased. Apologies if I came across otherwise.
We’re debating the wrong thing. The question isn’t whether the current text is wrong or incorrect; the question should be whether the new text is better. So let’s compare:
Old: The
.mjs
extension is especially useful on other platforms such as Node.js, where there’s no concept of MIME types or other mandatory hooks such astype="module"
to determine whether something is a module or a regular script.
New: The
.mjs
extension is especially useful on other platforms such as Node.js andd8
, where it’s a convenient way to indicate to the runtime that your code is a module.
Both sentences offer a reason why .mjs
is especially useful. The old sentence’s reason is why .mjs
is useful for platforms: .mjs
is useful for such platforms to have a way to be able to recognize modules. The new sentence’s reason is why .mjs
is useful for users: this is a convenient way to signal to platforms that a file is a module.
As this article is written for users, I think the new sentence is more appropriate.
Hi @mathiasbynens. In general, I removed the notes about Node.js because they were written years ago when As for your other questions about “why remove,” none of those other sentences were actually removed, they were just moved. I combined the two points about development into the same paragraph, etc. The diff makes things look like the changes were more dramatic than they were. |
It was never the case. But then again, the article doesn’t claim
I still think it’s more than we need. All the original sentences that are affected by this patch are still accurate as-is and convey the same information. It’s just that some links are outdated (we should point to https://nodejs.org/api/esm.html#esm_enabling nowadays, as @WebReflection pointed out). And we could mention Do you want to amend this patch accordingly, or would you prefer I suggest these as changes? |
I wrote up at the top what I’m trying to fix: The The article is very direct: “we recommend using the
In other words, the recommendation to use As I wrote in google/WebFundamentals#7623, there are significant groups of users who lack control over their server configuration: they’re using shared hosting, or delivering files for a client who will host them, or they’re publishing an open source library (or code within something like a WordPress theme) where the eventual hosting environment is unknown. For such users, Rather than amending the recommendation with caveats, I think it’s simpler and more equitable to just not make a recommendation at all. Say what |
The article already covers this:
…and:
Again, I don’t think this needs any changes, and I am not in favor of this patch in its current form, which removes the general recommendation. I would accept a patch making the updates explained above. |
I think it’s important to limit the general recommendation to development, and to make clear that production is a different story. Simply saying that extensions don’t matter to browsers and that servers need configuration is not the same as saying that some users should continue to use There is also an equity issue here. You and I might not know any developers who lack the luxury of full control of their servers, or professional systems engineering support, but there are millions of JavaScript developers out there scraping by without such privileges. I think part of the passion that the |
|
||
:::note | ||
**Note:** To deploy `.mjs` on the web, your web server needs to be configured to serve files with this extension using the appropriate `Content-Type: text/javascript` header, as mentioned above. Additionally, you may want to configure your editor to treat `.mjs` files as `.js` files to get syntax highlighting. Most modern editors already do this by default. | ||
**Note:** You may want to configure your editor to treat `.mjs` files as `.js` files to get syntax highlighting. Most modern editors already do this by default. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ve revised this to be much closer to your original text. I still think it weakens the article to include a discussion of non-Web platforms, but if you want that included, I think this language is a fairer and stronger explanation of .mjs
’s benefits than the previous text. Now that all ESM-supporting runtimes (both browsers and Node.js and d8 and Deno and Spidermonkey and JSC) support ESM in either .js
or .mjs
files, I think it’s more accurate to say that the benefit of .mjs
is that it’s a consistent way of declaring your code to be a module, in contrast to each runtime’s proprietary configuration. .mjs
is metadata that ships with the file itself, rather than nearby such as in a package.json
.
The previous previous text (“It’s consistent with Node.js, where the experimental modules implementation only supports files with the .mjs
extension.” before you added “by default” in response to "type": "module"
) seems like it was written back when it seemed like .mjs
was the only way that ESM was going to be supported in Node.js, and therefore it was really important to persuade the world to adopt .mjs
in order to help Node join the ESM bandwagon (and vice versa, to help prevent ESM adoption from being slowed by incompatibility with Node). I know you say that something like "type": "module"
was always the plan, and I’ve heard that from others in the Node modules group, but I’ve scoured the Internet for any record of such history and I can’t find any. I would be very curious to read any if you have any links you could share.
There is though: “On the web, file extensions don’t really matter […]”. (I serve my super old personal site’s scripts without any extensions, hah!) It feels like we keep going back and forth on this discussion rehashing the same arguments.
This whole discussion is about a side note on a feature explainer on the V8 website. We recommend modern JavaScript techniques all the time. It doesn’t mean we don’t care about users that need to support legacy browsers or enviroments. When we say “use |
@mathiasbynens How can we get this PR merged in? I understand where you’re coming from—the only thing “wrong” with the current text is some links that are outdated. Certainly, an argument can be made that the current article is otherwise correct, so why mess with it. What I’m proposing in this PR is to make it better. It seems like the recommendation for |
We still believe server software should feel encouraged to add first-class support for In order to resolve this, I'll tweak your patch based on my feedback throughout this discussion and merge it in. Thanks for your work on this! |
Resolves google/WebFundamentals#7623.
The
.mjs
extension is a perfectly valid recommendation for development, but users should not be encouraged to publish JavaScript files with an.mjs
extension unless they’re sure that such files will be served with the proper MIME type.