Update acorn, fix tests

[jsg2021]

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).

[jsg2021]

This resolves #879 and #866

[hroland]

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?

[jsg2021]

I ran the build script then tested. npm run build?

[matthewharwood]

@jsg2021 Any update on this?

[jsg2021]

I have not received any feedback or help.

[jsg2021]

@jdalton I'm willing to help with this, but I need some guidance on resolving the test failures.

[jsg2021]

Just tested master, and these same tests fail. Running tests on v3.2.25, has simular failures. (--check tests, and pm2 scenarios)

[jsg2021]

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().

[jsg2021]

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.

[jsg2021]

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.

[hroland]

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?

[jsg2021]

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 notifications@github.com 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 .

[antony]

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?

[jsg2021]

is lint not run as a check?

[antony]

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

[jsg2021]

I believe the errors in the test/fixtures files are intentional.

[jsg2021]

And double checked, and I haven't modified those files.

[antony]

Sorry, sent you the wrong one: https://github.com/beyonk-adventures/esm/runs/918092904?check_suite_focus=true

[jsg2021]

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

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?

[jsg2021]

I would fix the lint errors, but i hesitate to include orthogonal changes.

[antony]

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.

[hroland]

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 🙂

[matthewharwood]

Any kind of ETA on this the suspense is killing me lol

[jsg2021]

I haven't heard from any maintainer.:(

[jsg2021]

squashed the test fixes.

You can use this PR directly until it updates by using a url as your version spec. 1) Clone my fork 2) run npm install and pack:

# This will produce a tarball `./esm-3.2.25.tgz` that you can point to...
$ npm install && npm pack

3) Move the .tgz file to your project directory and rename to esm-3.x.x-pr883.tgz 4) Set esm's version to file://./esm-3.x.x-pr883.tgz in 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

[hroland]

Waiting eagerly :) @jdalton @benjamn

[Ghazay]

Waiting <3

[galvez]

Pinging @jdalton -- if you can find the time, we'll collectively owe you a beer.

[yurikoex]

squashed the test fixes.

You can use this PR directly until it updates by using a url as your version spec.

1. Clone my fork
2. run npm install and pack:

# This will produce a tarball `./esm-3.2.25.tgz` that you can point to...
$ npm install && npm pack

1. Move the .tgz file to your project directory and rename to esm-3.x.x-pr883.tgz
2. Set esm's version to file://./esm-3.x.x-pr883.tgz in 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 '.''

[jsg2021]

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.

[daveisfera]

@jdalton With node 14 hitting LTS soon, any chance we can get this merged and released?

[jsg2021]

@daveisfera It would be nice to have for sure. However, node 14 will ship with official first-pass esm support. It won't be a dropin replacement for webpack/rollup but you can largely write new code with its idioms today... and THOSE work in both.

[daveisfera]

Unfortunately, there are still some kinks being worked out and not all uses work yet (like this one)

[daveisfera]

Any chance of getting this merged since the built-in ESM support in node isn't quite as usable as esm is?

[hadyrashwan]

When can we expect this PR? as we use nullish coalescing in our code

[jsg2021]

I don't have control. There are still failing tests (related to node 13/14/15 repl)... @jdalton is a busy man and is very likely burned out with this project. I'd consider this project dead. My fork is open, feel free to fix the outstanding tests and maybe eventually someone will be given maintainer privileges.

I'm planning to phase out this project in my code and use native esm.