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

Clarify recommendations regarding .mjs #228

Closed
wants to merge 2 commits into from

Conversation

GeoffreyBooth
Copy link
Contributor

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.

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.
Copy link
Member

@mathiasbynens mathiasbynens left a 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?

src/features/modules.md Show resolved Hide resolved
src/features/modules.md Show resolved Hide resolved
src/features/dynamic-import.md Outdated Show resolved Hide resolved
@mathiasbynens mathiasbynens self-assigned this Aug 23, 2019
@mathiasbynens mathiasbynens added the language features Explainers for new JavaScript or WebAssembly features label Aug 23, 2019
@@ -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.
Copy link
Member

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.

Copy link
Contributor Author

@GeoffreyBooth GeoffreyBooth Aug 24, 2019

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.

Copy link
Member

@mathiasbynens mathiasbynens Aug 24, 2019

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’s type="module" isn’t mandatory either: a regular script can load a module via import(), with no type="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.

Copy link
Contributor Author

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 as type="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.

Copy link
Contributor Author

@GeoffreyBooth GeoffreyBooth Aug 24, 2019

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 as type="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 and d8, 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.

@GeoffreyBooth
Copy link
Contributor Author

GeoffreyBooth commented Aug 23, 2019

Hi @mathiasbynens. In general, I removed the notes about Node.js because they were written years ago when .mjs was going to be required to use ESM syntax in Node and that’s no longer the case. While .mjs is still supported, it’s one of two ways to enable ESM support for files in Node and it’s not favored or preferred over package.json "type": "module". On the contrary, the modules group has been discussing encouraging all public packages to include the type field, regardless of whether or not the files in the package use .js or .mjs. So any discussion of ESM support in Node really should include a mention of the type field; but then it starts to feel like a digression into ESM support in Node, which is somewhat off-topic for articles about using ESM syntax on the Web.

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.

@mathiasbynens
Copy link
Member

In general, I removed the notes about Node.js because they were written years ago when .mjs was going to be required to use ESM syntax in Node and that’s no longer the case.

It was never the case. .mjs was never going to be required in Node.js — the plan has always been to allow for configuration to override the default.

But then again, the article doesn’t claim .mjs is required, so I’m not really sure what this patch is trying to fix.

The diff makes things look like the changes were more dramatic than they were.

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 d8 as per @WebReflection’s request above.

Do you want to amend this patch accordingly, or would you prefer I suggest these as changes?

@GeoffreyBooth
Copy link
Contributor Author

the article doesn’t claim .mjs is required, so I’m not really sure what this patch is trying to fix.

I wrote up at the top what I’m trying to fix: 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. The recommendation is the issue I’m hoping to address.

The article is very direct: “we recommend using the .mjs extension for modules.” There are no qualifications; all users should use .mjs, period. There is no acknowledgement of use cases where .mjs would be inappropriate or not recommended; the closest the article comes is this sentence:

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.

In other words, the recommendation to use .mjs is so strong that the user should reconfigure their server in order to reap the benefits of .mjs; or presumably find a new host if they lack the ability to configure the current one. Using .js instead isn’t even mentioned as an option.

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, .js is the safest choice or possibly even the only option. By not acknowledging these users, in my opinion, the article implies that they’re doing something wrong (or worse, that they’re not worthy of acknowledgement: pity those poor plebeians who can’t configure their own servers!).

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 .mjs is and why it’s useful, and when it can and can’t be used, and leave it at that. The users who are in a position to be able to use it, and who find its advantages appealing, will use it; everyone else should feel fine using .js for ESM.

@mathiasbynens
Copy link
Member

The article already covers this:

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.

…and:

On the web, file extensions don’t really matter […]

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.

@GeoffreyBooth
Copy link
Contributor Author

I am not in favor of this patch in its current form, which removes the general recommendation.

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 .js in certain scenarios. A naïve user might read the article as it’s currently written and think, “well I use shared hosting where .mjs isn’t supported. I guess I can’t use ESM syntax.” There’s nothing in the article suggesting a solution that would work for them, and that’s why I find the current language misleading.

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 .mjs debate entails is indignation on the part of this population that they’re being advised to use this new thing, and therefore they’re backward if they’re not using it; but their economic situation is such that they don’t have the resources to use it. Especially coming from the ivory tower of Google, I think it’s important that Google make recommendations that apply to everyone and not just the privileged.


:::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.
Copy link
Contributor Author

@GeoffreyBooth GeoffreyBooth Aug 24, 2019

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.

@mathiasbynens
Copy link
Member

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 .js in certain scenarios. A naïve user might read the article as it’s currently written and think, “well I use shared hosting where .mjs isn’t supported. I guess I can’t use ESM syntax.” There’s nothing in the article suggesting a solution that would work for them, and that’s why I find the current language misleading.

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.

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 .mjs debate entails is indignation on the part of this population that they’re being advised to use this new thing, and therefore they’re backward if they’re not using it; but their economic situation is such that they don’t have the resources to use it. Especially coming from the ivory tower of Google, I think it’s important that Google make recommendations that apply to everyone and not just the privileged.

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 Map instead of plain objects [in situation $x]”, that’s not the same as recommending folks to drop support for environnments that lack Map support, and I don’t appreciate you implying that it is. Developers commonly use tooling such as transpilers and bundlers for their projects. The article we’re talking about explicitly mentions bundlers (and actively recommends the continued use of bundlers).

@GeoffreyBooth
Copy link
Contributor Author

@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 .mjs was written years ago when the only (public) plan for ESM support in Node.js was via the .mjs extension, and therefore it was important to persuade package authors to start using .mjs. Once Node.js added support for ESM via means other than .mjs, rather than substantially rewrite the article a few qualifiers were added to keep the text largely unchanged. While this may keep the article correct in a “precise” sense, I think the article could be improved further by reframing the argument; what was originally “Node needs .mjs” became “Node prefers .mjs”, but ultimately I think a better article would be “you should prefer .mjs, and here’s why (with caveats for when not to use it).” As in, put the users front and center rather than the Node.js development team, and also don’t omit important notes about when .mjs isn’t a good choice. That’s what I’m aiming for in this PR.

@mathiasbynens
Copy link
Member

We still believe server software should feel encouraged to add first-class support for .mjs, and weakening our recommendations in this way would go against that.

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes language features Explainers for new JavaScript or WebAssembly features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JavaScript modules note on file extensions is incorrect
4 participants