feat(eslint-plugin-sdk): add lint rules for Node-only globals and built-in imports - #2313
Conversation
…in function files Add two rules that report, in files defining a resolver, executor, workflow job, or HTTP adapter, what the build otherwise rejects only after bundling: - no-node-only-globals: references to the Node-only ambient globals the runtime never defines (process, Buffer, __dirname, require, ...). Locally bound names, property keys, type positions, and `typeof x` guards are ignored, mirroring the CLI's free-variable scan. - no-node-builtin-imports: static imports, re-exports, and static `import()` of Node built-in modules. Type-only imports are ignored; `require()` is left to the globals rule. Move the shared message tables and the Node-only global list into `@tailor-platform/shared/node-builtins` so the SDK and the plugin print the same suggestions; the SDK pins its `globals`-derived set to that list. In files that only define HTTP adapters the messages omit the suggestions, which are written for `body` functions. Enable both rules at warn in every create-sdk template.
…bals `typeof x !== "undefined" && typeof x.y !== "undefined" && x.y.z` and the `||` / ternary counterparts guard every operand that follows, so walk the logical chain instead of only its first operand.
…orts
- A `type` alias or `interface` sharing a Node-only global name does not
bind a value, so the global reference is still reported.
- `export { type X } from "node:fs"` is erased at runtime and is no longer
reported as a built-in import.
… ambient declarations
Sharing `isValueReference` had added `ExportSpecifier` to its exempt list,
which silenced `no-unconditional-permit` on an unsafe constant re-exported
with `export { permission }`. Treat the specifier's local side as a value
reference again and exempt only declaration binding names.
A `declare const process` erases at compile time, so it binds no runtime
value; `no-node-only-globals` now reports the global it appears to shadow.
… reference
`declare global {}` names the scope being augmented, so its `global`
identifier is not a reference to the Node global the runtime lacks.
🦋 Changeset detectedLatest commit: 53195cc The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@tailor-platform/create-sdk
@tailor-platform/eslint-plugin-sdk
@tailor-platform/sdk
@tailor-platform/sdk-plugin-seed
@tailor-platform/sdk-plugin-setup
@tailor-platform/sdk-plugin-tailordb-erd
commit: |
| `${RESOLVER}import { process } from "./pipeline";\nexport const run = () => process();`, | ||
| RULE, | ||
| ); | ||
| expectClean(`${RESOLVER}export const value = config.process.module;`, RULE); |
There was a problem hiding this comment.
The README states that a reference reaching a Node-only global through globalThis.process is intentionally not reported (README.md:287-288), and isValueReference() (lib/ast.ts) implements this by treating the property position of a non-computed MemberExpression as a non-value-reference.
That helper is shared across rules, so a future change to it could silently break this guarantee without any test failing.
Could you add a regression test for this case here, e.g.:
expectClean(`${RESOLVER}export const region = globalThis.process.env.REGION;`, RULE);
toiroakr
left a comment
There was a problem hiding this comment.
LGTM overall — see inline comment for a minor test-coverage nit.
The README documents that a reference reaching a Node-only global through `globalThis` is not reported, but no test pinned that behavior, so a change to the shared `isValueReference()` helper could have silently changed it.
Code Metrics Report (packages/sdk)
Details | | main (06957ca) | #2313 (b5db9aa) | +/- |
|--------------------|----------------|-----------------|-------|
- | Coverage | 83.5% | 83.5% | -0.1% |
| Files | 512 | 512 | 0 |
| Lines | 20664 | 20653 | -11 |
- | Covered | 17271 | 17260 | -11 |
+ | Code to Test Ratio | 1:0.5 | 1:0.5 | +0.0 |
| Code | 157022 | 156983 | -39 |
+ | Test | 82289 | 82293 | +4 |Code coverage of files in pull request scope (100.0% → 100.0%)
SDK Configure Bundle Size
Runtime Performance
Type Performance (instantiations)
Reported by octocov |
Summary
Two new lint rules report, in files that define a resolver, executor, workflow job, or HTTP adapter, the Node APIs the Tailor Platform runtime does not provide — previously rejected only after bundling.
Added rules (enabled at
warnin scaffolded projects)no-node-only-globals—process,Buffer,__dirname,require, and the other Node-only globals the runtime never definesno-node-builtin-imports— static imports, re-exports, and staticimport()ofnode:fs,path,crypto, and other built-in modulesExample
Both print the same suggested alternative the build does (
Use \defineConfig({ env })`…,Use the Web Crypto API…`). The message tables move to the internal shared package so the CLI and the plugin cannot drift.Notes
tailor.config.ts, scripts, and tests are unaffected. A Node API used in a helper module those files import is not reported; the build still checks the bundled output.typeof xguards, andglobalThis.processare not reported, matching the CLI's free-variable scan.bodyfunctions.