feat(remix-dev/compiler): support native `.node` files

[nicksrandall]

trafficstars

This PR enables support for users to include "native" (.node) modules into their server bundles (when using NodeJS).

Libraries like sharp (awesome for for image processing) are written in a lower-level language than NodeJS and then are compiled as a "native" module to be used in a Node environment.

Discussion: #2560

• [ ] Docs
• [ ] Tests

[remix-cla-bot[bot]]

Hi @nicksrandall,

Welcome, and thank you for contributing to Remix!

Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once.

You may review the CLA and sign it by adding your name to contributors.yml.

Once the CLA is signed, the CLA Signed label will be added to the pull request.

If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at [email protected].

Thanks!

- The Remix team

[remix-cla-bot[bot]]

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: 2fec55f4d40c8f2700d325f9b111626286369f15

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[machour]

Sorry @nicksrandall, I did a whopsy while trying to change the base branch (code changes need to target dev). I'll get this fixed unless you do.

[benwis]

@MichaelDeBoey I don't particularly care which PR gets merged, but I have no ability to edit this here. I think you need the node system check, the compiler changes, the path detection code, and everything else I had to do in my PR. We're running this change in Prod right now to serve .node files, and it seems to work well.

[nicksrandall]

@benwis I'll give you access to my branch. I also don't care which PR gets merged so if it's easier to close then and re-open yours, we can do that.

[benwis]

@nicksrandall Thanks for inviting me! I'm happy to do it here, I'll dig in and address the comments today. I'd love to see this and the WASM changes merged!

[MichaelDeBoey]

Let's keep this branch open then 👍

[benwis]

@MichaelDeBoey @nicksrandall I have updated this repo for the changes in Remix 1.7.1 and reapplied the changes made, plus some of mine. I'm not sure how to get this pull request to update based on the status of the dev branch of the parent repo though. It seems to be stuck.

[nicksrandall]

@MichaelDeBoey any updates on this. This is a blocker for me.

[MichaelDeBoey]

@nicksrandall Please rebase your branch onto latest dev & resolve conflicts I'll inform the team to have a look at this afterwards

[chaance]

@nicksrandall I'd like to get this merged but there are comments left by @jacob-ebey that still haven't been addressed, and the server compiler code was moved into compilerServer.ts so some of the code needs to be moved over. If you can get this cleaned up and address outstanding issues, I'll merge.

[nicksrandall]

I'll look at this today and see if I can get it updated.

[nicksrandall]

@chaance The solutions here has gotten more simple since esbuild added support for the copy loader.

See: https://github.com/evanw/esbuild/issues/1051#issuecomment-1156783762

This should just work when app imports a .node file directly.

One potential gotcha that I feel like I should point out is that packages that ship .node files have to compile a node file per OS/Architecture (like linux/amd64). Sometimes, the library author will then use dynamic require statements to require the appropriate node file. Esbuild can not know the path of a dynamic require statement so this will not work in those cases.

In such a case, the user will just need to mark the package as "external" (or in Remix case, exclude it from being bundled) and then it should just work.

[nicksrandall]

@benwis I know you also worked on this... You comfortable just using the copy loader for .node files?

[chaance]

In such a case, the user will just need to mark the package as "external" (or in Remix case, exclude it from being bundled) and then it should just work.

I think that's probably fine for now. The same is true for any other dynamic file import.

[benwis]

I think that's probably fine as well, although I haven't found time to test it :)

[github-actions[bot]]

🤖 Hello there,

We just published version v0.0.0-nightly-66b33a8-20230310 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

[MoSattler]

I am still running into this issue when trying to install adminjs.

✘ [ERROR] No loader is configured for ".node" files: node_modules/fsevents/fsevents.node

node_modules/fsevents/fsevents.js:13:23:
  13 │ const Native = require("./fsevents.node");

serverDependenciesToBundle is not fixing this

serverDependenciesToBundle: ["fsevents"]