exercism / javascript

Exercism exercises in JavaScript.
https://exercism.org/tracks/javascript
MIT License
569 stars 613 forks source link

CI all green even if package.json is not in sync #1557

Open junedev opened 2 years ago

junedev commented 2 years ago

We noticed the master got out of sync somehow and here in my PR https://github.com/exercism/javascript/pull/1476, the package.json is not in sync but the CI is all green nevertheless. If I run the sync command, I get a change for the package.json file.

cc @SleeplessByte @tejasbubane

junedev commented 2 years ago

So this is the error message:

Checking integrity of concept/windowing-system/package.json...
Error [ERR_REQUIRE_ESM]: require() of ES Module /home/runner/work/javascript/javascript/node_modules/chalk/source/index.js from /home/runner/work/javascript/javascript/scripts/checksum not supported.
Instead change the require of index.js in /home/runner/work/javascript/javascript/scripts/checksum to a dynamic import() which is available in all CommonJS modules.
    at Object.newLoader [as .js] (/home/runner/work/javascript/javascript/node_modules/pirates/lib/index.js:104:7)
    at checksumAssignment (/home/runner/work/javascript/javascript/scripts/checksum:43:19)
    at /home/runner/work/javascript/javascript/scripts/checksum:105:14
    at Array.every (<anonymous>)
    at checksumAll (/home/runner/work/javascript/javascript/scripts/checksum:103:24)
    at Object.<anonymous> (/home/runner/work/javascript/javascript/scripts/checksum:126:1)
    at Object.newLoader [as .js] (/home/runner/work/javascript/javascript/node_modules/pirates/lib/index.js:104:7)
    at Object.<anonymous> (/home/runner/work/javascript/javascript/node_modules/@babel/node/lib/_babel-node.js:176:21) {
  code: 'ERR_REQUIRE_ESM'
}

But for some reason that does not fail the CI job.

The root cause is probably one of the dependabot updates that got merged.

junedev commented 2 years ago

This is the cause: https://github.com/exercism/javascript/pull/1541 Chalk was updated to a new major version that only supports ESM now.

junedev commented 2 years ago

I am reverting the chalk version now as a quick fix but there are still other todos here:

SleeplessByte commented 2 years ago

Yep. Your investigation is correct.

junedev commented 2 years ago

CI is fixed now, my PR correctly shows up as out of sync. image

Judging by my twitter feed ESM/CommonJS is a hot topic in the whole ecosystem. But I didn't understand how to best deal with it for now.

SleeplessByte commented 2 years ago

It's one of the things where I just hack around it too. I have a few projects with esm-only packages, so we can likely copy that configuration to make things work again. Feel free to pick that up if you want to, but otherwise I will get to it, before the year turns 2022.

junedev commented 2 years ago

Would be great if you could take care of the rest of this issue. I just assigned myself for doing the quick fix.

SleeplessByte commented 2 years ago

Yes!