Skip to content

Add ESLint rule blocking Node built-in imports in core/#2229

Open
rafal-hawrylak wants to merge 1 commit into
mainfrom
add-eslint-no-node-builtins
Open

Add ESLint rule blocking Node built-in imports in core/#2229
rafal-hawrylak wants to merge 1 commit into
mainfrom
add-eslint-no-node-builtins

Conversation

@rafal-hawrylak

Copy link
Copy Markdown
Collaborator

Files under core/ run inside a V8 compilation sandbox on GCP that does not provide Node.js built-in modules. Nothing at PR time currently enforces this - a top-level import * as nodePath from "path" added to core/session.ts in 3.0.53 broke every cloud compilation for ~7 days.

Adds a custom ESLint rule no-node-builtins that fails on any import of a Node built-in under core//*.ts. The blocklist is derived at rule load time from require('module').builtinModules, covering both bare (path) and prefixed (node:path) forms, so future Node built-ins are caught automatically. Files matching core//*_test.ts are excluded via ESLint overrides since tests run on the host Node runtime.

Wires eslint into scripts/run_tests alongside the existing tslint check, so PR CI blocks the regression class at merge time. tslint remains authoritative for the rest of the repo.

@rafal-hawrylak
rafal-hawrylak requested a review from a team as a code owner July 22, 2026 11:27
Files under core/ run inside a V8 compilation sandbox on GCP that does
not provide Node.js built-in modules. Nothing at PR time currently
enforces this - a top-level `import * as nodePath from "path"` added to
core/session.ts in 3.0.53 broke every cloud compilation for ~7 days.

Adds a custom ESLint rule `no-node-builtins` that fails on any import
of a Node built-in under core/**/*.ts. The blocklist is derived at rule
load time from `require('module').builtinModules`, covering both bare
(`path`) and prefixed (`node:path`) forms, so future Node built-ins
are caught automatically. Files matching core/**/*_test.ts are excluded
via ESLint `overrides` since tests run on the host Node runtime.

Wires eslint into scripts/run_tests alongside the existing tslint
check, so PR CI blocks the regression class at merge time. tslint
remains authoritative for the rest of the repo.
@rafal-hawrylak
rafal-hawrylak force-pushed the add-eslint-no-node-builtins branch from 147f596 to 2cd7ce9 Compare July 22, 2026 11:46
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.

1 participant