Skip to content

refactor: npm rpc: use old school require instead of the experimental json import#5628

Merged
link2xt merged 2 commits intomainfrom
simon/replace-experimental-json-import-replaced-by-old-school-require
Jun 7, 2024
Merged

refactor: npm rpc: use old school require instead of the experimental json import#5628
link2xt merged 2 commits intomainfrom
simon/replace-experimental-json-import-replaced-by-old-school-require

Conversation

@Simon-Laux
Copy link
Copy Markdown
Member

to get rid of warning. Should also make it possible to use nodejs versions older than 20.11.

Disclaimer: needs to be tested, I haven't tested it yet.

@Simon-Laux Simon-Laux requested a review from link2xt May 31, 2024 19:02
@link2xt
Copy link
Copy Markdown
Collaborator

link2xt commented Jun 5, 2024

Can remove the note "The minimum nodejs version for this package is 20.11" from the readme as well.

@Simon-Laux
Copy link
Copy Markdown
Member Author

Can remove the note "The minimum nodejs version for this package is 20.11" from the readme as well.

There should still be a minimum version. maybe 16 or 18 then. but 20 is current LTS version anyway.

@link2xt
Copy link
Copy Markdown
Collaborator

link2xt commented Jun 6, 2024

There should still be a minimum version. maybe 16 or 18 then. but 20 is current LTS version anyway.

@hpk42 has node 16 and other than this problem it worked (after removing this line directly in node_modules). So it can be stated that minimum supported is 16.

We don't test this package with CI so it is not really ensured, would be nice to use this in deltachat-jsonrpc/typescript/ tests instead of compiling the binary with a direct call to cargo and using it from target here:
https://github.com/deltachat/deltachat-core-rust/blob/b6dceb42716ce1557a87c491469b578cb75c91dc/deltachat-jsonrpc/typescript/test/test_base.ts#L17
But otherwise I think it's fine to state that we at least intend to support node 16.

@Simon-Laux
Copy link
Copy Markdown
Member Author

Simon-Laux commented Jun 6, 2024

@hpk42 has node 16

he should update, also electron uses a newer version internally.

@link2xt
Copy link
Copy Markdown
Collaborator

link2xt commented Jun 6, 2024

@hpk42 has node 16

he should update, also electron uses a newer version internally.

The bot (https://github.com/link2xt/xdcterm) does not need electron and otherwise works fine with node 16.

@link2xt link2xt marked this pull request as ready for review June 7, 2024 21:33
@link2xt link2xt merged commit d412887 into main Jun 7, 2024
@link2xt link2xt deleted the simon/replace-experimental-json-import-replaced-by-old-school-require branch June 7, 2024 21:34
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

Successfully merging this pull request may close these issues.

2 participants