-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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 ES Modules recipe #1593
Add ES Modules recipe #1593
Conversation
Recipe for ava + @std/esm + typescript
docs/recipes/[email protected]
Outdated
"cjs": true, | ||
"await": true, | ||
"gz": true | ||
} |
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 don't believe all those options are needed. Can you try snipping it down to something like:
"@std/esm": "cjs"
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.
@jdalton Sure.
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.
Sweet 🍬 !
Thanks for starting this @motss. I've tweaked the text and simplified parts of it a little bit. Can you also add it to the AVA readme: https://github.com/avajs/ava#recipes |
docs/recipes/[email protected]
Outdated
|
||
However, it works a bit different on Node.js than in web browsers that also natively supports ES modules. In Node.js, you need to use the `.mjs` extension instead of the commonly known `.js` extension. The `.mjs` extension in web browsers too, but they have to be served with the correct media type (`application/javascript`). There is a [ongoing effort](https://tools.ietf.org/html/draft-bfarias-javascript-mjs-00) on standardizing `.mjs`. | ||
|
||
The good news is that there's a module called [`@std/esm`](https://github.com/standard-things/esm) that enable you to write and run ES modules in Node.js 4 and above. |
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.
that enable -> that enables
docs/recipes/[email protected]
Outdated
``` | ||
|
||
You can now write tests with AVA, `@std/esm`, and TypeScript: | ||
|
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.
👆 So rad!
docs/recipes/[email protected]
Outdated
```json | ||
{ | ||
… | ||
"moduleResolution": "node", |
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.
👆 is the ...
in the json common in recipe docs? It makes a weird error highlight when using ```json
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.
It's just a way to signify that there should be more content than just this in the file. Any better suggestion?
Let me know if a specific usage pops out for AVA. |
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.
Thanks @motss!
Some minor text comments (perhaps those were @sindresorhus' changes). Would be neat if this could show an example of using .mjs
files, too.
docs/recipes/[email protected]
Outdated
|
||
As of Node.js 8.5.0, [ES modules](http://2ality.com/2017/09/native-esm-node.html) are natively supported, but behind a command line option `--experimental-modules`. | ||
|
||
However, it works a bit different on Node.js than in web browsers that also natively supports ES modules. In Node.js, you need to use the `.mjs` extension instead of the commonly known `.js` extension. The `.mjs` extension in web browsers too, but they have to be served with the correct media type (`application/javascript`). There is a [ongoing effort](https://tools.ietf.org/html/draft-bfarias-javascript-mjs-00) on standardizing `.mjs`. |
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 think this should say:
The
.mjs
extension can work in web browsers too, but such files have to be served with the correct media type
And:
There is an ongoing effort on standardizing
.mjs
.
(an
rather than a
)
docs/recipes/[email protected]
Outdated
```js | ||
import test from 'ava'; | ||
|
||
test('2 + 2 = 4', t => { |
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.
It'd be nice if this would have an actual example of a .mjs
file. Notably the test files are still transpiled using Babel, which means the above really reads const test = require('ava')
, and there are no .mjs
files being used. Same goes for the TypeScript example below.
I'm fine not dwelling on mjs. The recommended config is "@std/esm":"cjs", which enables cjs compat mode. |
docs/recipes/[email protected]
Outdated
"test": "ava" | ||
}, | ||
"dependencies": { | ||
"@std/esm": "^0.16.0" |
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.
Latest is 0.17.0
now. Is there a standard way to express a dependency without using a version for the recipe so folks don't lazy copy pasta an out of date version?
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 don't know… perhaps the recipe should say to use npm install
and not show any package.json
? That would make copying really hard.
@sindresorhus @jdalton Does the PR look good? Ready to merge? |
See #1618 |
What do we do about this PR now that #1618 is merged? |
The general advice is still solid. Maybe bump the version documented and run with it. |
You might want to update this PR to point out that Running
This was surprising to me since the documentation in this PR mentions the |
The |
* support @std/esm * fix path test for windows * fix linter error * Stricter regex. Use shorthand .esmrc file in fixture. * update package-lock.json
I've pushed a rewrite that focuses this recipe on using @jdalton is the configuration OK, and the example accurate? |
I've pushed a rewrite that focuses this recipe on using @std/esm
, with AVA 1.0 in mind.
Looks great! Let's roll with that configuration and loosen it if needed later. |
} | ||
``` | ||
|
||
By default AVA converts ES module syntax to CommonJS. [You can disable this](./babel.md#preserve-es-module-syntax). |
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.
You don't need to prefix the file path with ./
. It works without too.
Thanks all! |
🎉 Thanks for merging the PR. Well done all! 👏 |
@sindresorhus
To fix #1590
Recipe for ava + @std/esm + typescript