Skip to content
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

NodeJS 22 issues/feedbacks tracker #87

Open
robertsLando opened this issue Sep 10, 2024 · 39 comments
Open

NodeJS 22 issues/feedbacks tracker #87

robertsLando opened this issue Sep 10, 2024 · 39 comments

Comments

@robertsLando
Copy link
Member

robertsLando commented Sep 10, 2024

NodeJS 22 support has been added starting from pkg 5.14.0 but many things have changed on NodeJS source and so also on our patch file so it needs some tests. Use this issue to submit your feedbacks and issues

KNOWN ISSUES:

cc @faulpeltz

@robertsLando robertsLando pinned this issue Sep 10, 2024
@robertsLando robertsLando changed the title NodeJS 22 issues tracker NodeJS 22 issues/feedbacks tracker Sep 10, 2024
@faulpeltz
Copy link

I managed to complete a Node 22 build with macos arm64 after removing the android SDK (the change from the node/build repo), and then also removed the iOS simulator cache (which is nearly 50GB)

yao-pkg/pkg-fetch@main...faulpeltz:pkg-fetch:main#diff-a1654f4357833dfc4d0c6a8d39c2e3ee30ceea604c29238846903993434ab384R93

@robertsLando
Copy link
Member Author

@faulpeltz That's super! PR? 🚀

@robertsLando
Copy link
Member Author

I would extend that also on x64 build to prevent any future issue with this :)

@faulpeltz
Copy link

@faulpeltz That's super! PR? 🚀

I'm trying to find out why the x64 macos build on -13 is so fragile, I had it crash or timeout multiple times. But the cleanup seems not necessary for now because it has ~200GB free disk space, and I assume it will imrpove in the future 🤷‍♂️

I really don't want to complain about free Apple build infrastructure, but 14GB of free disk space is a bit of a stretch...

@robertsLando
Copy link
Member Author

I really don't want to complain about free Apple build infrastructure, but 14GB of free disk space is a bit of a stretch...

Yeah definetely 😆 Also because storage nowdays doesn't cost that much, RAM and CPU does...

@cage1618
Copy link

Can I use this release in production now?

@robertsLando
Copy link
Member Author

@cage1618 If you are speaking about nodejs 22 I would say nope, but you can use nodejs 18/20 safely with latest release

@faulpeltz
Copy link

@robertsLando The Node 22 patch applies to the just-released 22.9 version without problems and all builds went through

In this release they disabled Maglev (one of the JIT compilers) because of reported crashes
https://github.com/nodejs/node/releases/tag/v22.9.0

So random crashes might be caused by latent Node 22 issues before 22.9, might make sense to bump the Node 22 version (but not urgent)

@robertsLando
Copy link
Member Author

@faulpeltz thanks for the info, I have an action to automatically create the patch, let me run it 🚀

@robertsLando
Copy link
Member Author

I have created a workflow that automatically check for new nodejs versions: https://github.com/yao-pkg/pkg-fetch/actions/workflows/check-latest-node.yml

@cage1618
Copy link

release 5.15.0 is stable now?

@balisaurus
Copy link

Using 5.15.0 to use latest nodejs 22 doesn't seem to work for my project.
I create a simple test code with a single file writing hello to console:

pkg --debug hello.js --targets node22-win-x64
> [email protected]
(node:27328) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
> [debug] Bytecode of D:\temp\testnode\hello.js is added to queue.
> [debug] Stat info of D:\temp\testnode\hello.js is added to queue.
> [debug] Directory D:\temp\testnode is added to queue.
> [debug] Stat info of D:\temp\testnode is added to queue.
> [debug] Directory D:\temp is added to queue.
> [debug] Stat info of D:\temp is added to queue.
> [debug] Directory D:\ is added to queue.
> [debug] Stat info of D:\ is added to queue.
> [debug] Deleting record file : D:\
> [debug] The file was included as bytecode (no sources)
  D:\temp\testnode\hello.js
> [debug] The file was included as bytecode (no sources)
  D:\temp\testnode\hello.js
> [debug] files & folders deduped =
  hello.js
> [debug] The directory files list was included (1 item)
  D:\temp\testnode
> [debug] The directory files list was included (1 item)
  D:\temp\testnode
> [debug] files & folders deduped =
  testnode
> [debug] The directory files list was included (1 item)
  D:\temp
> [debug] The directory files list was included (1 item)
  D:\temp
> [debug] Targets:
  [
  {
    "nodeRange": "node22",
    "platform": "win",
    "arch": "x64",
    "output": "d:\\temp\\testnode\\hello.exe",
    "forceBuild": false,
    "fabricator": {
      "nodeRange": "node22",
      "platform": "win",
      "arch": "x64",
      "binaryPath": "C:\\Users\\tekno\\.pkg-cache\\v3.5\\fetched-v22.9.0-win-x64"
    },
    "binaryPath": "C:\\Users\\tekno\\.pkg-cache\\v3.5\\fetched-v22.9.0-win-x64"
  }
]

Running it:

hello.exe
------------------------------- virtual file system
C:\snapshot
  testnode                                29 Bytes
    hello.js                              29 Bytes
Total size =  29 Bytes
node:internal/modules/cjs/loader:1254
  throw err;
  ^

Error: Cannot find module 'C:\snapshot\testnode\hello.js'
    at Module._resolveFilename (node:internal/modules/cjs/loader:1251:15)
    at Function._resolveFilename (pkg/prelude/bootstrap.js:1955:46)
    at Module._load (node:internal/modules/cjs/loader:1077:27)
    at Function.runMain (pkg/prelude/bootstrap.js:1983:12)
    at node:internal/main/run_main_module:30:49 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}

Node.js v22.9.0

@robertsLando
Copy link
Member Author

@balisaurus does it work with others nodejs version?

@BeneRasche
Copy link

Same problem here:

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

Error: Cannot find module 'C:\snapshot\appname\dist\apps\server\main.js'
at Module._resolveFilename (node:internal/modules/cjs/loader:1251:15)
at Function._resolveFilename (pkg/prelude/bootstrap.js:1955:46)
at Module._load (node:internal/modules/cjs/loader:1077:27)
at Function.runMain (pkg/prelude/bootstrap.js:1983:12)
at node:internal/main/run_main_module:30:49 {
code: 'MODULE_NOT_FOUND',
requireStack: []
}

Node.js v22.9.0


works with -t node20-win-x64 but not node22

@faulpeltz
Copy link

Seems that the snapshot fs doesn't work for module loading on Windows in the Node 22 patch

@robertsLando
Copy link
Member Author

@BeneRasche The window issue related to nodejs 22 should be fixed on next release, please be patient. Thanks to @faulpeltz for the patch!

@thisjt
Copy link

thisjt commented Sep 30, 2024

We're waiting for this, right?

@faulpeltz Unfortunately we need to wait a new nodejs 22 release in order to test this as I have no way with current workflows to force build a specific version, I need to implement this

Based on how often nodejs releases, I think it will be either next week or mid October

@robertsLando
Copy link
Member Author

robertsLando commented Sep 30, 2024

@thisjt yep, I'm sorry!

Problem is that actually I cannot release patches for nodejs patches as them would become broke for actual pkg versions as the shas are shipped with npm so updating them would change them and work only for latest vesrsions. I already have a workflow that runs every day at midnight and opens a PR when a new patch is available

@thisjt
Copy link

thisjt commented Oct 7, 2024

@robertsLando nodejs had a release 4 days ago. I think it's time to patch 😬 let's gooo- or does it need to have a v22 release?
https://github.com/nodejs/node/releases/tag/v20.18.0


Oh lol nvm my bad it needs a v22 release

@robertsLando
Copy link
Member Author

robertsLando commented Oct 8, 2024

@thisjt thanks for letting me know! Seems my automated workflows picked up it correctly 4 days ago but the patches do not apply cleanly, let me try to fix this

https://github.com/yao-pkg/pkg-fetch/actions/runs/11171900799/job/31057489528

BTW seems only node 20 has a new release for now

@thisjt
Copy link

thisjt commented Oct 18, 2024

node v22.10 is finally out! https://github.com/nodejs/node/releases/tag/v22.10.0

And with that node v23 is also out, hoping that we will also be supporting this. Not in a hurry tho, we're not sure yet on how stable v23 is.

@dingiyan
Copy link

I often follow this library and hope to keep up with the latest version of Node.js. This morning, I tested the v22 version, but it is not yet available. I hope excellent authors can continue to follow up and look forward to the latest available v22 version. Finally, I would like to thank the contributors of yao-pkg for their hard work

@robertsLando
Copy link
Member Author

New version is coming soon. The average delay I would like to keep between nodejs releases and pkg support for them is around 1 week, it depends on how many breaks there are on patches.

@robertsLando
Copy link
Member Author

pkg 5.16.0 is out now with node 20.18.0 and 22.10.0 support

@BeneRasche
Copy link

pkg-fetch probably needs an update for 22.10 binaries, too?

@robertsLando
Copy link
Member Author

@BeneRasche It's out already!

@BeneRasche
Copy link

@BeneRasche It's out already!

Where?

https://github.com/yao-pkg/pkg-fetch/releases

@robertsLando
Copy link
Member Author

2024-10-21_15-36

@dingiyan
Copy link

dingiyan commented Oct 22, 2024

Hello, @robertsLando
I found a bug on node v22. This issue seems to be caused by the inconsistent behavior of pkg's require
for example, the streamx package was required

// index.js
const sss = require("streamx")

console.log(sss.isStreamx);

on node v22, I use node index.js , and the result is ok.

on pkg build (22.10 version, target is windows), the exe excuted but print an error, like this:


D:\study\demo\pkg-22>index.exe
pkg/prelude/bootstrap.js:1872
      throw error;
      ^

Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: No "exports" main defined in C:\snapshot\pkg-22\node_modules\streamx\package.json
    at exportsNotFound (node:internal/modules/esm/resolve:296:10)
    at packageExportsResolve (node:internal/modules/esm/resolve:586:13)
    at resolveExports (node:internal/modules/cjs/loader:644:36)
    at Function._findPath (node:internal/modules/cjs/loader:752:31)
    at Function.<anonymous> (node:internal/modules/cjs/loader:1239:27)
    at Function._resolveFilename (pkg/prelude/bootstrap.js:1947:44)
    at Function._load (node:internal/modules/cjs/loader:1079:27)
    at TracingChannel.traceSync (node:diagnostics_channel:315:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:218:24)
    at Module.<anonymous> (node:internal/modules/cjs/loader:1344:12) {
  code: 'ERR_PACKAGE_PATH_NOT_EXPORTED'
}

Node.js v22.10.0

I found, the steramx package has main field for export the package entry, but not exports field.
and the node22 changed the require method for support require(esm)

I think, the problem is pkg's require method not equal to node22's require method.
It's like the pkg's require take for the module is esm (but it's a cjs module ) and throws error.

please check it, thank you.

@robertsLando
Copy link
Member Author

robertsLando commented Oct 22, 2024

@dingiyan thanks for your detailed issue! Could you tell me if that's just related to 22-win? Are you also able to reproduce it with 22-linux? I think this may be related to:

https://github.com/yao-pkg/pkg-fetch/blob/main/patches/node.v22.10.0.cpp.patch#L344-L346

@faulpeltz what do you think?

There is also a reference to https://github.com/yao-pkg/pkg/blob/main/prelude/bootstrap.js#L1942

But I don't think the issue is there

@robertsLando
Copy link
Member Author

Someone wants to give this a try? #110 🚀

@dingiyan
Copy link

@dingiyan thanks for your detailed issue! Could you tell me if that's just related to 22-win? Are you also able to reproduce it with 22-linux? I think this may be related to:

https://github.com/yao-pkg/pkg-fetch/blob/main/patches/node.v22.10.0.cpp.patch#L344-L346

@faulpeltz what do you think?

There is also a reference to https://github.com/yao-pkg/pkg/blob/main/prelude/bootstrap.js#L1942

But I don't think the issue is there

I tested on Mac, and got same error. I think linux is too.
I think the issue point is node.v22.10.0.cpp.patch

@faulpeltz
Copy link

@dingiyan thanks for your detailed issue! Could you tell me if that's just related to 22-win? Are you also able to reproduce it with 22-linux? I think this may be related to:

https://github.com/yao-pkg/pkg-fetch/blob/main/patches/node.v22.10.0.cpp.patch#L344-L346

@faulpeltz what do you think?

There is also a reference to https://github.com/yao-pkg/pkg/blob/main/prelude/bootstrap.js#L1942

But I don't think the issue is there

I can reproduce it on Linux, and I'm working on a fix

@robertsLando
Copy link
Member Author

Ok will be fixed on next node 22 release 🙏🏼

@robertsLando
Copy link
Member Author

The alternative is to delete binaries from release and redo the release now

@faulpeltz
Copy link

Would it be possible to create a new GH release for each pkg fetch release and keep the new binaries there?

@robertsLando
Copy link
Member Author

Would it be possible to create a new GH release for each pkg fetch release and keep the new binaries there?

That's actually not possible as pkg downloads assets from pkg-fetch GH releases.

@thisjt
Copy link

thisjt commented Oct 22, 2024

Maybe we can add a feature wherein we can specifically target a pkg-fetch release instead of getting the latest one for that specific node version. That way we'll have some flexibility on what pkg-fetch release we can specifically target instead of always getting the latest one.

I'd need to look into how the entire pkg-fetch works as a whole though so I might need a bit of schooling as to why this is, or why this is not possible or viable. Would be nice to have this feature so that we can roll out hotfixes instead of waiting for a new node release every time we need to fix an issue.

@robertsLando
Copy link
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants