Skip to content

Conversation

@nickodell
Copy link
Contributor

Update NCC; pin fetch to v2

Reference Issue

#70

Reasoning behind change

I found that when using the version of NCC installed by package.json, the compiler is not capable of understanding many of the constructs within this code and its dependencies. For example, I get this error:

Module parse failed: Unexpected character '#' (13:2)
File was processed with these loaders:
 * ./node_modules/@zeit/ncc/dist/ncc/loaders/empty-loader.js
 * ./node_modules/@zeit/ncc/dist/ncc/loaders/relocate-loader.js
 * ./node_modules/@zeit/ncc/dist/ncc/loaders/shebang-loader.js
You may need an additional loader to handle the result of these loaders.
|    * @type {Map<string, import('./cache').requestResponseList}
|    */
>   #caches = new Map()

The version of NCC that the package.json refers to is no longer supported upstream. Instead, use @vercel/ncc to provide this.

I also found that it was necessary to pin node fetch to a pre v3 version. If this is not done, I get the following error when running the action:

node:internal/modules/cjs/loader:1215
  throw err;
  ^

Error: Cannot find module 'node-fetch'
Require stack:
- /home/runner/work/_actions/nickodell/circleci-artifacts-redirector-action/e396aa885116bf6964236ba34707f520d11d0034/dist/index.js
    at Module._resolveFilename (node:internal/modules/cjs/loader:1212:15)
    at Module._load (node:internal/modules/cjs/loader:1043:27)
    at Module.require (node:internal/modules/cjs/loader:1298:19)
    at require (node:internal/modules/helpers:182:18)
    at 5266 (/home/runner/work/_actions/nickodell/circleci-artifacts-redirector-action/e396aa885116bf6964236ba34707f520d11d0034/dist/index.js:29215:33)
    at __nccwpck_require__ (/home/runner/work/_actions/nickodell/circleci-artifacts-redirector-action/e396aa885116bf6964236ba34707f520d11d0034/dist/index.js:31099:43)
    at /home/runner/work/_actions/nickodell/circleci-artifacts-redirector-action/e396aa885116bf6964236ba34707f520d11d0034/dist/index.js:31127:15
    at Object.<anonymous> (/home/runner/work/_actions/nickodell/circleci-artifacts-redirector-action/e396aa885116bf6964236ba34707f520d11d0034/dist/index.js:31216:12)
    at Module._compile (node:internal/modules/cjs/loader:1529:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1613:10) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/home/runner/work/_actions/nickodell/circleci-artifacts-redirector-action/e396aa885116bf6964236ba34707f520d11d0034/dist/index.js'
  ]
}

Node.js v20.19.1

As far as I can tell, this is caused by node-fetch@v3 being ESM-only, where we use a CommonJS style import.

I have not re-generated dist/index.js using these settings. I assume that since this is essentially an object file, you would prefer to have a trusted maintainer do this. If you prefer that I do this step, I can add this to the PR.

@larsoner
Copy link
Collaborator

I have not re-generated dist/index.js using these settings. I assume that since this is essentially an object file, you would prefer to have a trusted maintainer do this. If you prefer that I do this step, I can add this to the PR.

I'll see if I can create an autofix CI to do this, unless you know a better way!

@bsipocz bsipocz added the dependencies Pull requests that update a dependency file label May 19, 2025
@bsipocz
Copy link
Member

bsipocz commented May 19, 2025

I'll see if I can create an autofix CI to do this, unless you know a better way!

An autofix CI job would be awesome as there have been a couple of dependabot updates to the lockfile, too that have not resulted in a index.js update.

@larsoner
Copy link
Collaborator

@nickodell if you're motivated to try, you could add a version of this job (can be in this PR):

https://github.com/mne-tools/mne-installers/blob/main/.github/workflows/outdated.yml

If not I'll do it tomorrow hopefully

@nickodell
Copy link
Contributor Author

@nickodell if you're motivated to try, you could add a version of this job (can be in this PR):

I'm afraid I'm not motivated. I am already three layers deep into yak shaving, and I don't want any new yaks.

@larsoner
Copy link
Collaborator

Good bot, thanks for shaving that yak 🐮 (closest emoji I could think of to a yak!)

In it goes, thanks @nickodell !

@larsoner larsoner merged commit bc4ba3e into scientific-python:master May 20, 2025
3 checks passed
larsoner added a commit to matthieutrs/circleci-artifacts-redirector-action that referenced this pull request May 20, 2025
* upstream/master:
  Update NCC; pin fetch to v2 (scientific-python#71)
  ENH: Add automatic rebuild (scientific-python#72)
  Build(deps): Bump undici from 5.28.5 to 5.29.0
  Making config example and narrative consistent (scientific-python#67)
@bsipocz
Copy link
Member

bsipocz commented May 20, 2025

I'm just generating noise, sorry about it. But I really like the yak analogy and will start using it myself as it's a nice counterpart for the new puppies of newly added features,

(what about 🐂🏔️?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants