Stop transpiling ESM out of the box?
novemberborn opened this issue · 15 comments
Currently we transpile ESM syntax in test files. This has frequently caused confusion when users also rely on that syntax in their source files, but are unclear how to configure AVA to transpile sources. #1905 (comment) being the most recent example.
Further, we only transpile syntax, without implementing the specced behavior. Now that esm is available that is probably the better choice for most users. However I don't think it's something we should default to.
I wonder if it would be clearer to not transpile ESM syntax, as well as remove it from our examples. We can then more clearly communicate the available options for supporting ESM:
- Configure AVA's Babel pipeline to convert to CJS, which introduces users to the Babel pipeline and lets us more clearly signal how to configure source compilation
- Configure
esm
We could consider exposing esm as ava/esm, since we already have a dependency on it. Similarly we could implement the ESM-to-CJS compilation through our ava/stage-4 plugin. I'm leaning towards making people explicitly install esm or the @babel/plugin-transform-modules-commonjs packages.
It'll also be easier to support Node.js experimental ESM support, especially now that we support custom file extensions.
What do folks think?
I wouldn't mind too much either way, but I will say that for me personally, AVA was a nice, practical introduction to ES6 and ESM. There are a lot of blog posts talking about ESM, but it barely has any usage yet outside of projects with a Babel build step. With AVA, it feels native even though it's not, since it's zero-config for test files, and I like that. It feels like the future (and it is).
I've always found it pretty easy to test CJS modules with AVA. I think testing Babel projects is a bit harder, but that seems expected to me.
As for how to reduce the mental burden on users, I have a few ideas:
- I think this is a great case for a flow chart. AVA basically has two different setup flows, depending on whether your project needs Babel or not. That lends itself well to a visual explanation in a diagram and I think it's a good way to express that you should make a choice from the beginning.
create-avashould detect my usage of Babel and configure AVA based on that. For example, put@babel/registerinava.requirein package.json for me. And output some friendly messages related to Babel setup. Featurecreate-avamore prominently in the README, recipes, and blog posts. Get people to use it and lean on its automation.- Official Yeoman generators for both Babel and non-Babel "AVA Hello World" setups would be super useful to people.
I don't think it's worth breaking pretty much everyone's tests just because some users don't understand how it works. The solution here is better docs.
I do agree the esm module is a better than transpiling the syntax with Babel, but I don't think it's worth this big of a breakage.
Configure AVA's Babel pipeline to convert to CJS, which introduces users to the Babel pipeline and lets us more clearly signal how to configure source compilation
I was hoping AVA would rather move to a model where it either transpiles everything (or the specified files to be transpiled) or nothing. That way there's no confusion.
Thanks @sholladay. I agree with all your points.
For Babel projects, I agree that the biggest hurdle is that AVA doesn't compile sources. Especially now that new language features are adopted more quickly this leads to projects that may only be compiled for distribution, either using Babel or webpack. Users get syntax errors when they import non-compiled source files.
For projects using webpack, the extra difficulty is that, if they're also compiled using Babel, that Babel configuration typically does not compile ESM, since that's left to webpack. AVA's test-file only support for ESM can be hugely misleading.
For projects using esm, there's the counter-intuitive step of having to disable AVA's ESM compilation.
The solution here is better docs.
We could revisit our recipes with these project types in mind. We could also detect these syntax errors and point users to the relevant documentation.
I was hoping AVA would rather move to a model where it either transpiles everything (or the specified files to be transpiled) or nothing. That way there's no confusion.
Note though that if user compiles ESM syntax through a different tool (webpack, or esm) AVA won't pick up on this automatically.
I do agree the
esmis a better than transpiling the syntax with Babel, but I don't think it's worth this big of a breakage.
Perhaps we need to make it easier to opt-out of AVA's default ESM handling. Right now it involves specifying our ava/stage-4 preset and disabling module compilation. Perhaps it should be a flag, e.g. package.json#ava.babel.compileEsm: Boolean. If we do that, perhaps we could default to compiling ESM even for sources.
I could be 👍 for this change if we made the breakage worth it:
- Disable transpilation by default unless there's a
babelconfig.
I want to remove the performance hit of Babel for users that don't use it and improve stack traces. - Remove Babel as a direct dependency.
We would use the Babel version used by the project if it's in the range supported by AVA. - Don't transpile ESM by default when Babel is used.
- When Babel is used, transpile everything by default, not just tests.
Maybe this needs to wait for AVA v2, but I would still like to do all of this in one go to reduce churn for users.
Yes that makes sense. I'd like something similar for TypeScript.
I was thinking to have @ava/babel and @ava/typescript packages, which are required as soon as you enter configuration for babel and typescript. They themselves could have peer dependencies on AVA and Babel / TypeScript respectively.
Agreed that this is for after 1.0.
I've been trying out ava for the first time today, and can confirm that I totally expected it to transpile my imported files in addition to my test files. I'd be fine with it either transpiling all by default, or none by default (with a single option to enable transpilation of all) as long as it's consistent.
In the interim, though, this worked perfectly fine:
package.json
"ava": {
"require": "esm"
}
It might be a good idea to explicitly point out in the readme under Transpiling imported modules that this is the simplest solution (until a change is made for v2). Until I found this issue, it wasn't very clear what I needed to do to fix the problem
To add my 2p, I never expected ava to transpile my imported files in addition to my test files and documentation told that somewhere. @chrisdothtml you're a rare case :)
Personally I found the ava's recipe about bypassing the ESM transpiling a little outdated (talking about Node 8) and a bit hit-and-miss in general. Things are explained but partially. However, I tried out all setups until one worked and settled with that. There's no nyc mentioned there!
All my libraries' sources are in ES6 and I use Rollup to transpile to ESM+CJS+UMD and the main reason behind pointing ava towards ESM build rather than CJS is coverage.
If transpiled CJS files are used to calculate coverage, all functions that Babel adds are left out and the scores suffer (not to mention CJS code lines differ in Coveralls.io from ESM source, for example).
It's ideal when source is ESM and ava runs on ESM build (no transpiling) and coverage is calculated on ESM build.
Sadly, since Babel 7 the usual nyc --require esm --reporter=html --reporter=text ava" does not work and I had to temporarily remove the coverage completely from all my libraries until I find the way and/or dependencies catch up with Babel 7.
Looking at higher level, Rollup does the transpiling and it references the Babel config in library's root. I've got @babel/core as dev-dependency. But then, nyc pipes ava through esm — the whole existence of esm seems questionable to me — in theory, why can't all the parts of the code use the same @babel/core from node_modules and use @babel/preset-env referenced in library's root? Why do we even need esm? But that's more stones to nyc rather than ava's yard. You guys are doing great and I look forward to proper, semantic 1.0.0 release.
Agreed. Really like ava, but when trying to bypass transpiling (node.js v8+) none of the recipes and answers really worked here, only a "mix-and-match" combo. In the (eventually) working solution test and source files (using es6 import/export) have to have the .mjs extension, esm must be installed as part of the dev-dependencies, no ava config file or .babelrc file is present, the ava config in package.json must require esm, the babel/extensions need to include js and mjs and module compilation needs to be disabled for testOptions/presets:
"ava": { "require": "esm", "babel": { "extensions": [ "js", "mjs" ], "testOptions": { "presets": [ ["module:ava/stage-4", {"modules": false}] ] } } }
It would be great to clean up/clarify the documentation and add specific (verified working) sample skeleton test projects for the most important es6 settings/scenarios for the 1.0.0 final release.
Remove Babel as a direct dependency. (from #1908 (comment))
This would be delightful for smaller projects, the only reason I'm still choosing tape over ava is the 13 MB install size difference.
the only reason I'm still choosing tape over ava is the 13 MB install size difference
As long as it is a devDependency, the size won't affect your users except during development. Also, I'm willing to bet that if you do npm ls ava, you'll find it in your dependency tree already. So npm should be able to dedupe it and install only once.
It does affect the developer experience though and npm ls ava certainly does not return in my dependency tree for most projects.
Regular user here (with no knowledge of Babel or AVA internals).
I like having my builds completely separate from my tests. That's why I have a babelrc that transpiles sources+tests to an out-dir first. Then I run AVA on the out-dir with require: 'esm', babel: false and compileEnhancements: false. This works really well for me. I can even run babel --watch and ava --watch side by side and I haven't had any issues with that so far (but more experimentation is needed).
edit: getting code coverage to work without polluting the transpiled source files is somewhat of an issue with this setup
edit2: I decided not to have test coverage because of reasons outlined here https://medium.com/the-node-js-collection/rethinking-javascript-test-coverage-5726fb272949
edit3: c8 looks like a promising alternative to nyc that doesn't pollute build output, but it's currently broken on my Windows machine
Our setup uses Rollup to bundle ECMAScript Modules into ESM and ES5 (for unsupported browsers), and we want our tests to also be written in ESM.
After a lot of trial and error the only solution that worked was the one in this comment #1908 (comment)
Please see #2293.