chore(ok): forbid require() in test files via oxlint (#2762)#724
Merged
Conversation
* chore(lint): forbid require() in test files, convert existing to ESM
vitest runs each test file as an ES module where a CommonJS require() is
not defined, so require('./foo.ts') and inline require('node:fs') break or
resolve inconsistently at runtime (dima flagged this after the vitest
migration). Add an oxlint no-restricted-syntax rule scoped to *.test.* that
forbids require() calls, and convert every existing occurrence.
- oxlint.config.ts: test-file override banning CallExpression require(),
with a message pointing at import / await import / createRequire
- server/app/desktop test files: inline require() to static ESM imports or
already-imported bindings
- native-binding tests: rename the createRequire const to require_ (matches
the existing convention) so the ban has no false positives while keeping
the sanctioned native-addon load
require() calls inside Bun.spawnSync / node -e child-script string literals
are untouched: they run in a spawned CommonJS process and are not AST
require() calls, so the rule correctly does not flag them.
* chore(lint): close require-ban coverage gaps from local review
- extend the glob to *.test.mjs (packages/desktop/vitest.config.ts already
discovers **/*.test.mjs, and .mjs is inherently ESM where require() throws)
- broaden the selector to also match member calls like require.resolve(...),
which are equally undefined in an ESM test module
- reword the message + comment to be runner-neutral (files still import the
bun:test shim during the vitest transition) and to name the require_
convention for createRequire bindings
* fix(cli): load native addon via static import so knip sees the dep
The createRequire + require_ rename dodged the oxlint require ban, but knip
detects a dependency reference by the same require() callee name it keys on,
so require_('@inkeep/open-knowledge-native-config') left knip unable to see
the dep and it failed the lint gate with "Unused devDependencies". Replace
createRequire with a static `import * as nativeConfig` (knip-visible, no
require at all, still fails loud at module load if the addon is unbuilt) and
assign it to the existing local binding interface — zero call-site churn.
Verified: cli typecheck clean, both native suites green (18 tests), oxlint
clean on both files, and pnpm run knip exits 0.
* chore(test): migrate touched test files from bun:test to vitest
Rebasing onto main picked up the incremental no-new-bun-test-imports guard
(#2757), which forbids `bun:test` imports in any file a change touches. This
PR modifies these 12 test files for the require->ESM conversion, so each must
also drop the bun:test shim import.
All 12 use only describe/test/expect + beforeEach/afterEach — none of the
divergent shim APIs (mock.module / jest / setSystemTime / setDefaultTimeout /
test.if / vi) — so this is a straight `from 'bun:test'` -> `from 'vitest'`
specifier swap. Verified: the guard passes, lint + typecheck clean, and all
299 tests across the touched files stay green.
* docs(lint): clarify require-ban comment + drop stale native-test parity claims
Address review suggestions:
- oxlint.config.ts: reword the escape-hatch comment so "both" has explicit
referents (both selectors key on the literal name `require`).
- native binding tests: drop the "matching toml-config-engine.test.ts" /
"matching the sibling binding tests" claims — after the switch to a static
`import * as`, those siblings load via a different mechanism; keep the
fail-loud rationale, which is the actual point.
GitOrigin-RevId: 713f9a49ab52de4df131cc60a8a8705e533b69a8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Copybara-translated 1 Inkeep OSS change. Rebase-merge this PR so the prepared commit lands directly on public main.