refactor: replace NodeJS with NodeJS_18_jll #1069
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR replaces NodeJS.jl with the JLL package NodeJS_18_jll.jl built from Yggdrasil.
This also indirectly fixes an issue with NodeJS.jl not declaring cxxstring_abi.
Note 1. I tested the PR on my personal website (source) and it prerendered pages with both KaTeX and highlight.js.
Note 2. The diff on
Project.toml
is misleadingly large since it re-ordered some entries.In order to update
Project.toml
, I performed the following REPL commands.Note 3. There is a slightly higher maintenance burden as the jll will have to be manually updated to new major versions.
Note 4. This PR isn't critical to getting my website to build since there's the
$NODE
environmental variable escape hatch that I discovered while implementing this PR. Perhaps there should be a little note in the documentation or the README about this, but then again, missing the C++ standard library on Linux is pretty niche.Note 5. I implemented the PR with the following substitutions
NodeJS
->NodeJS_18_jll
nodejs_cmd()
->node()
$(npm_cmd())
->$(node()) $npm
Note that a strict, literal replacement would look like
nodejs_cmd()
->`$(node().exec[1])`
npm_cmd()
->`$(node().exec[1]) $npm`
However, in the context of a
run(...)
it's possible to dorun(`$(nodejs_cmd()) $arguments`)
->run(`$(node()) $arguments`)
run(`$(npm_cmd()) $arguments`)
->run(`$(node()) $npm $arguments`)
motivating the simpler implementation.
Note that the difference is nontrivial, as the BinaryBuilder.jl documentation explains:
Indeed, using the literal substitutions breaks when it can't find the shared libraries,
which is also why
$npm
cannot be used directly, as it needs to be innode()
's environment.Here are examples of all the listed substitutions.
Note 6. The
run(`$(node()) $arguments`)
syntax requires Julia v1.6+.If compatibility with old Julia versions is important, the legacy syntax
can be used instead.