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

Add .mjs to the examples #683

Merged
merged 1 commit into from
Sep 22, 2018
Merged

Add .mjs to the examples #683

merged 1 commit into from
Sep 22, 2018

Conversation

philipwalton
Copy link
Contributor

As developers are starting to publish .mjs files to npm, people are starting to see breakage in unexpected ways (e.g. this tweet) due to the fact that most babel-loader users copy and paste the test: /\.js$/ line into their config.

While we can't update everyone's existing configs, we can update the canonical examples.

/cc @mathiasbynens @developit

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

Does this PR introduce a breaking change?

  • Yes
  • No

As developers are starting to publish `.mjs` files to npm, people are starting to see breakage in unexpected ways (e.g. [this tweet](https://twitter.com/capajj/status/1042836215022735361)) due to the fact that most `babel-loader` users copy and paste the `test: /\.js$/` line into their config.

While we can't update everyone's existing configs, we can update the canonical examples.

/cc @mathiasbynens @developit
philipwalton added a commit to philipwalton/babel-loader that referenced this pull request Sep 21, 2018
@loganfsmyth
Copy link
Member

loganfsmyth commented Sep 21, 2018

I admit, I'm very on the fence here because it seems like publishing .mjs currently is actively hostile in 99% of cases because there's no guarantee that .mjs files publishes now would be compatible with Node's eventual implementation. The only safe .mjs implementation right now would be one that has 0 external dependencies, or at least 0 external CommonJS deps. It seems like anything encouraging .mjs support should be shunned until Node settles on interop behavior.

@loganfsmyth
Copy link
Member

I guess at the end of the day it's not really our call how people do this, but boy does it seem like a terrible idea at the moment.

@philipwalton
Copy link
Contributor Author

philipwalton commented Sep 21, 2018

I've never been crazy about the extension name myself, but since all modern browsers already do ship a module implementation that actually runs code differently from how it runs regular scripts, I think it's wise to recommend developers use a different extension in order to express the intent of where the code will be run.

My colleague @mathiasbynens wrote an article promoting .mjs (and he may have more thoughts here) so more people are starting to use this, and unfortunately many of them are hitting issues like this and getting confused.

I've added a note to one of my blogs posts around the potential gotchas of .mjs, so hopefully that will help as well: https://philipwalton.com/articles/deploying-es2015-code-in-production-today/#mjs-warnings

screen shot 2018-09-21 at 3 53 48 pm

@loganfsmyth
Copy link
Member

loganfsmyth commented Sep 21, 2018

Yeah, I'm actually 100% for .mjs as a format, my concern is more that tooling adopting it without rules for how interop works seems dangerous. If Webpack itself for instance only allowed .mjs files to import other ESM files, I'd have zero concerns.

In the end though, I do think this PR is something we should just land, I just feel that the need for this PR indicates further pain for the community in the future, since people are going to adopt it expecting it to interop with CommonJS nicely, and that's still entirely undefined beyond experimental implementations.

Users are going to be pissed if all their code stops working in some future Webpack version.

@philipwalton
Copy link
Contributor Author

I think those are very valid concerns. And I agree future pain seems likely ... I'm just hoping we can minimize it as much as possible.

@mathiasbynens
Copy link

because there's no guarantee that .mjs files publishes now would be compatible with Node's eventual implementation.

The higher-order bit is that not using .mjs is not compatible with Node’s current implementation.

Modulo some proposals that were explicitly rejected (e.g. .m.js), there’s no guarantee about anything regarding Node’s eventual implementation, as it hasn’t been figured out yet. If I had to guess, I’d think it’s more likely that .mjs will continue to be supported by Node.

I think this discussion is orthogonal to this PR, though.

@loganfsmyth loganfsmyth merged commit c8d7a72 into babel:master Sep 22, 2018
willmendesneto pushed a commit to willmendesneto/feature-toggle-service that referenced this pull request Sep 30, 2018
## The devDependency [babel-loader](https://github.com/babel/babel-loader) was updated from `7.1.5` to `8.0.3`.
This version is **not covered** by your **current version range**.

If you don’t accept this pull request, your project will work just like it did before. However, you might be missing out on a bunch of new features, fixes and/or performance improvements from the dependency update.

---

<details>
<summary>Release Notes for v8.0.3</summary>

<h3>Features</h3>
<ul>
<li><a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="363280376" data-permission-text="Issue title is private" data-url="babel/babel-loader#687" href="https://urls.greenkeeper.io/babel/babel-loader/pull/687">#687</a> - Add <code>customize</code> option</li>
</ul>
<h3>Bugs</h3>
<ul>
<li><a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="362860284" data-permission-text="Issue title is private" data-url="babel/babel-loader#685" href="https://urls.greenkeeper.io/babel/babel-loader/pull/685">#685</a> - Also pass the caller option to loadPartialConfig</li>
</ul>
<h3>Docs</h3>
<ul>
<li><a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="361610577" data-permission-text="Issue title is private" data-url="babel/babel-loader#681" href="https://urls.greenkeeper.io/babel/babel-loader/pull/681">#681</a> - Update the README links to use the new options docs</li>
<li><a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="362787813" data-permission-text="Issue title is private" data-url="babel/babel-loader#683" href="https://urls.greenkeeper.io/babel/babel-loader/pull/683">#683</a> - Add .mjs to the examples</li>
</ul>
<h3>Internal</h3>
<p>Some dev dependency updates and CI tweaks.</p>
</details>

<details>
<summary>Commits</summary>
<p>The new version differs by 77 commits ahead by 77, behind by 9.</p>
<ul>
<li><a href="https://urls.greenkeeper.io/babel/babel-loader/commit/800181b91dc4136bdb1d068f1fe730998eac6512"><code>800181b</code></a> <code>8.0.3</code></li>
<li><a href="https://urls.greenkeeper.io/babel/babel-loader/commit/7d8500ced72046d4c5c1dcdc7a9c0016d7e6c15b"><code>7d8500c</code></a> <code>Also pass the caller option to loadPartialConfig (#685)</code></li>
<li><a href="https://urls.greenkeeper.io/babel/babel-loader/commit/a50791472f1f706bf2ec4ea06427df2dd8b31080"><code>a507914</code></a> <code>Expose the full loader options to all overrides hooks.</code></li>
<li><a href="https://urls.greenkeeper.io/babel/babel-loader/commit/ac0c869b86b98881ac5544bf7248c63a971b7219"><code>ac0c869</code></a> <code>Tweak the customize implementation to be a bit more strict.</code></li>
<li><a href="https://urls.greenkeeper.io/babel/babel-loader/commit/9b70a02b3069000133122c09271ce0a7144f8dae"><code>9b70a02</code></a> <code>Add overrides option</code></li>
<li><a href="https://urls.greenkeeper.io/babel/babel-loader/commit/c8d7a7287a6b0dc60480b026159f0486ac9bf074"><code>c8d7a72</code></a> <code>Add .mjs to the examples (#683)</code></li>
<li><a href="https://urls.greenkeeper.io/babel/babel-loader/commit/461999323585fdaa420530a9f85ff5701a7d298e"><code>4619993</code></a> <code>bable options link update (#681)</code></li>
<li><a href="https://urls.greenkeeper.io/babel/babel-loader/commit/8f240b498bb24ef89f7b306f5ac806e84b813b0d"><code>8f240b4</code></a> <code>Use node 10 on appveyor</code></li>
<li><a href="https://urls.greenkeeper.io/babel/babel-loader/commit/7e4189ebb6e313df460de25949503e73cb260bf6"><code>7e4189e</code></a> <code>Change appveyor to use babel account</code></li>
<li><a href="https://urls.greenkeeper.io/babel/babel-loader/commit/eeaee463232e786d030716894ed4f66e505acbde"><code>eeaee46</code></a> <code>Update devDeps to use most recent versions, and fix tests.</code></li>
<li><a href="https://urls.greenkeeper.io/babel/babel-loader/commit/3e5fb5e3ab9bd02d6955625bde4c71fee1619473"><code>3e5fb5e</code></a> <code>chore(package): update lockfile</code></li>
<li><a href="https://urls.greenkeeper.io/babel/babel-loader/commit/2b8e479e50ce6dc7fc0bc5ae700951b9631a4959"><code>2b8e479</code></a> <code>chore(package): update eslint-config-babel to version 8.0.0</code></li>
<li><a href="https://urls.greenkeeper.io/babel/babel-loader/commit/0e43d0ab51d3b3f627e39d82e85b40a99c6316e3"><code>0e43d0a</code></a> <code>8.0.2</code></li>
<li><a href="https://urls.greenkeeper.io/babel/babel-loader/commit/5e0565d1199bcf42c238657f48460727680a2864"><code>5e0565d</code></a> <code>Manually fix 'inline' sourcemaps so they work with Webpack. (#671)</code></li>
<li><a href="https://urls.greenkeeper.io/babel/babel-loader/commit/ea52d05a41c4b44288348093ac3bb7ef52fa60bc"><code>ea52d05</code></a> <code>Use 'sourceMaps' since that is what we suggest to use in our docs. (#670)</code></li>
</ul>
<p>There are 77 commits in total.</p>
<p>See the <a href="https://urls.greenkeeper.io/babel/babel-loader/compare/f3bfed932a959f7ffe3633e7f88cba0f3fcbf7b2...800181b91dc4136bdb1d068f1fe730998eac6512">full diff</a></p>
</details>

<details>
  <summary>FAQ and help</summary>

  There is a collection of [frequently asked questions](https://greenkeeper.io/faq.html). If those don’t help, you can always [ask the humans behind Greenkeeper](https://github.com/greenkeeperio/greenkeeper/issues/new).
</details>

---


Your [Greenkeeper](https://greenkeeper.io) bot 🌴
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

Successfully merging this pull request may close these issues.

3 participants