esm
esm copied to clipboard
standard-things
Update acorn, fix tests
Updated: Updating acorn adds support for the language features users have been requesting. It works. However, running the tests against the current-generation node is proving challenging. All tests pass on node 11 (at least locally), on node 12+ things break subtly. There is one test that is probably a legitimate failure. Mocha is trying to diff a "module" and its function to normalize values doesn't have a case for 'module' and is defaulting to coercing to a string... which blows up. I'm not sure when the "module" object type was hardened, but that caused moduleObject + '' to throw a type error. Also, node 12 seems to have changed how you hook into --check or removed it... because those tests are failing only in node 12 and up.
The remainder of the failures are from running the tests on node 12 or newer. Dependencies that have type: module in their package.json brake the current usage of require(thing).
I can't seem to get this fork to work, neither with node -r esm, nor with initializing via main.js and index.js :(
Always hitting this error, even tried a fresh project:
<project path>/esm-test/node_modules/esm/esm.js:156
const { dir } = shared.package
^
TypeError: Cannot destructure property 'dir' of 'shared.package' as it is undefined.
at Object.<anonymous> (/<project path>/node_modules/esm/esm.js:156:11)
at Module._compile (internal/modules/cjs/loader.js:1158:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:1178:10)
at Module.load (internal/modules/cjs/loader.js:1002:32)
at Function.Module._load (internal/modules/cjs/loader.js:901:14)
at Module.require (internal/modules/cjs/loader.js:1044:19)
at require (internal/modules/cjs/helpers.js:77:18)
at Object.<anonymous> (/<project path>/index.js:3:11)
at Module._compile (internal/modules/cjs/loader.js:1158:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:1178:10)
Any ideas maybe?
@jdalton I'm willing to help with this, but I need some guidance on resolving the test failures.
Just tested master, and these same tests fail. Running tests on v3.2.25, has simular failures. (--check tests, and pm2 scenarios)
I've fixed all errors on node 11. There still are errors on node 12+. Looks like Node12+ broke the --check support. :/ The other failures are from attempting to require() a "module" and node is blowing up saying to use import().
Locally on node 11, using npm ci to install deps, tests all pass. The tests that are failing on CI and I'm not sure why.
I've worked through many of the failures. Should be good to go.
Side thought... the tests run on node configured in the build matrix. However, it also builds on those same instances. With newer dev deps cutting off at 10, it would probably be better to isolate the build and tests. So we can validate the compiled module works on node 6+ and not be required to build on less than node 10.
Great work, I hope it gets merged soon!
The reason that caused my error was when I added the package to an existing project via yarn add jsg2021/esm --latest, the build script didn't run. After cd ./node_modules/esm && yarn && yarn build, running node -r esm . worked!
Any thoughts why build doesn't run after adding the package?
the build doesn’t run because dev deps aren’t installed unless you run npm/yarn install from that package. esm is a zero-dep library. it must be built and published to be used.
On Fri, Jul 24, 2020 at 5:34 AM Roland Horváth [email protected] wrote:
Great work, I hope it gets merged soon!
The reason that caused my error https://github.com/standard-things/esm/pull/883#issuecomment-656574002 was when I added the package to an existing project via yarn add jsg2021/esm --latest, the build script didn't run. After cd ./node_modules/esm && yarn && yarn build, running node -r esm . worked!
Any thoughts why build doesn't run after adding the package?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/standard-things/esm/pull/883#issuecomment-663475616, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEM7LJBN3VQLB7M54UTSGLR5FPSLANCNFSM4OUDF2FQ .
This is great - thanks for your hard work.
One thing I noticed is that linting isn't passing any more.
/home/runner/work/esm/esm/src/hook/vm.js
##[error] 240:17 error Setter cannot return a value no-setter-return
##[error] 247:13 error Setter cannot return a value no-setter-return
/home/runner/work/esm/esm/test/fixture/import/const.mjs
##[error] 3:1 error 'value' is read-only no-import-assign
/home/runner/work/esm/esm/test/fixture/import/let.mjs
##[error] 3:1 error 'value' is read-only no-import-assign
✖ 4 problems (4 errors, 0 warnings)
This would probably need to be fixed in order for this to be merged?
It doesn't look like it, from studying the travis and appveyor config, it's omitted. I believe it is run by husky on the author's machine before push.
However I run it on ci for consistency, and it makes the build fail: https://github.com/beyonk-adventures/esm/actions/runs/185440336
Sorry, sent you the wrong one: https://github.com/beyonk-adventures/esm/runs/918092904?check_suite_focus=true
It doesn't look like it, from studying the travis and appveyor config, it's omitted. I believe it is run by
huskyon the author's machine before push.However I run it on
cifor consistency, and it makes the build fail: https://github.com/beyonk-adventures/esm/actions/runs/185440336
The error in that run is the EPRIVATE refusing to publish private package? The stacktrace is just a Module Concatenation bailout notice. I'm not seeing the lint errors?
Perhaps they're not important and already existed. Just thought I'd highlight them in case they would block merge for the PR. Since CI doesn't appear to depend on them, w can leave it to the package owners to decide the importance I guess.
the build doesn’t run because dev deps aren’t installed unless you run npm/yarn install from that package. esm is a zero-dep library. it must be built and published to be used. …
Ahh, thanks for letting me know! I learned something today 🙂
squashed the test fixes.
You can use this PR directly until it updates by using a url as your version spec.
- Clone my fork
- run npm install and pack:
# This will produce a tarball `./esm-3.2.25.tgz` that you can point to...
$ npm install && npm pack
- Move the .tgz file to your project directory and rename to
esm-3.x.x-pr883.tgz - Set esm's version to
file://./esm-3.x.x-pr883.tgzin your package.json and reinstall your node_modules.
Or if you trust public urls (I wouldn't but I'm offering for simplicity) you can set the version url to:
https://github.com/jsg2021/esm/releases/download/v3.x.x-pr883/esm-3.x.x-pr883.tgz
Pinging @jdalton -- if you can find the time, we'll collectively owe you a beer.
squashed the test fixes.
You can use this PR directly until it updates by using a url as your version spec.
- Clone my fork
- run npm install and pack:
# This will produce a tarball `./esm-3.2.25.tgz` that you can point to... $ npm install && npm pack
- Move the .tgz file to your project directory and rename to
esm-3.x.x-pr883.tgz- Set esm's version to
file://./esm-3.x.x-pr883.tgzin your package.json and reinstall your node_modules.Or if you trust public urls (I wouldn't but I'm offering for simplicity) you can set the version url to:
https://github.com/jsg2021/esm/releases/download/v3.x.x-pr883/esm-3.x.x-pr883.tgz
how would I activate optional chaining with this? Still seeing 'SyntaxError: Unexpected token '.''
this doesn't enable syntax. It only prevents the library from blowing up if used on a version of node that supports it and the code uses it.
node 14 has it enabled by default. earlier versions have it behind a flag.