aspect-build / aspect-build/rules_js

[FR]: Allow using node headers for node-gyp from repo rule

Open
#2,032 2 comments 0 reactions 0 assignees View on GitHub
enhancement help wanted need: funding
Dominant language
Starlark
Stars
378
Forks
183
Avg merge
1d 9h
Merged PRs (30d)
32

Description

### What is the current behavior?

## Background
At the moment, when running life cycle via `node-gyp`, it need to download node headers from https://nodejs.org/download/release/v20.12.2/node-v20.12.2-headers.tar.gz, which
1. Does not use bazel downloader
2. The url is external

## Use local tarball
`node-gyp` does provide some args that allows us to override the node header in many ways, and I think the most appropriate one is `tarball` https://github.com/nodejs/node-gyp?tab=readme-ov-file#command-options. It's configurable via `npm_config_tarball` env which can be achieved in `lifecycle_hooks_envs`.
What's the alternative?
1. `--dist-url`. It solves the external url issue, but it's still better to use the bazel downloader I think
2. `--devdir` and `--ensure`: `node-gyp` will check a file called `installVersion` under `devdir` and it's coupled with `node-gyp` version

## Blockers so far
In order to use `npm_config_tarball`, an intended workflow would be adding a repo rule, such as
```
http_archive(
name = "node_header",
strip_prefix = "node-v20.12.2",
urls = [
"https://nodejs.org/download/release/v20.12.2/node-v20.12.2-headers.tar.gz",
],
)
```
However, there's no way to pass the path to the lifecycle action at the moment if I get it correct?

### Describe the feature

I'm happy to create a PR for that, but I'm not sure what's the best solution. From what I can see there are multiple ones

## Pass tarball to `npm_translate_lock` directly

### API:

It can be called something like `lifecycle_hooks_gyp_tarball`, e.g. `lifecycle_hooks_gyp_tarball = "@node_header//tarball"`.

### Implementation:

When generating the `defs.bzl` via template for packages that need `node-gyp`, like `/external/aspect_rules_js~~npm~pnpm_deps__node-pty__1.0.0__links/defs.bzl`, `@node_header//tarball` is added to its `srcs`, and a flag will also be passed to `lifecycle-hooks`, `--tarball=xxx`. `lifecycle-hooks.js` will then append it with `process.cwd()` and pass to `node-gyp`

### pros

API is easy to use

### cons

A bit too specific? Not sure whether we want to introduce such an option

## Allow adding dependencies to lifecycle hook

### API
Add a new option maybe called `lifecycle_hooks_additional_deps`, like
```
"lifecycle_hooks_additional_deps": {
"node-pty": ["@node_header//tarball"]
}
```

### Implementation

When generating the `defs.bzl` via template for packages that need `node-gyp`, like `/external/aspect_rules_js~~npm~pnpm_deps__node-pty__1.0.0__links/defs.bzl`, these additional deps are added to its `srcs`. However, since bazel doesn't allow finding the absolute path of the input, we still need to find a way to prepend cwd in `lifecycle-hooks.js`. I'm thinking maybe we can add a functionality in `lifecycle-hooks.js`, to replace `$CWD` with `process.cwd()`?

## cons
A bit hard to consumes it, but I think it's fine? It's advance usage to use your own node header?

## pros
It's much generous, people may want to pass other deps to lifecycle hooks?

## Use `@rules_nodejs//nodejs/headers:current_node_cc_headers`
I'm blocked on the generated `defs.bzl`
```
_js_run_binary(
name = "{}/lc".format(store_target_name),
srcs = [
...
# node header tarball for node_gyp
"@rules_nodejs//nodejs/headers:current_node_cc_headers", <--
],
# js_run_binary runs in the output dir; must add "../../../" because paths are relative to the exec root
args = [
...
--header_dir=??? <-- how can I pass the dir to @rules_nodejs//nodejs/headers:current_node_cc_headers?
] +
select({
Label("@aspect_rules_js//platforms/os:osx"): ["--platform=darwin"],
Label("@aspect_rules_js//platforms/os:linux"): ["--platform=linux"],
Label("@aspect_rules_js//platforms/os:windows"): ["--platform=win32"],
"//conditions:default": [],
}) +
....
)
```

Contributor guide

Open the contributing guide

Research direction

Start by tracing npm_translate_lock and the generated defs.bzl for packages using node-gyp, then inspect lifecycle-hooks.js and the existing lifecycle hook environment handling. Compare the proposed tarball, additional-dependency, and current_node_cc_headers approaches. Done means a documented repo-rule workflow can provide local Node headers to node-gyp without its external download.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, nodejs
Domain
build-system, tooling
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.