Skip to content

fix: read the oxlint config, and run the rule it declares - #2432

Open
mhalikosen wants to merge 2 commits into
nestjs:masterfrom
mhalikosen:fix/oxlint-config-not-read
Open

fix: read the oxlint config, and run the rule it declares#2432
mhalikosen wants to merge 2 commits into
nestjs:masterfrom
mhalikosen:fix/oxlint-config-not-read

Conversation

@mhalikosen

Copy link
Copy Markdown

PR Checklist

The existing tests in application.factory.test.ts already assert the generated file list and read the config back, so they cover the rename; they are updated to the new name rather than duplicated.

PR Type

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior?

Issue Number: #2431

oxlint.json is never read. oxlint auto-discovers .oxlintrc.json, .oxlintrc.jsonc, oxlint.config.ts and oxlint.config.mts; oxlint.json is not one of them, and the generated lint script passes no -c. Every generated project therefore ships a lint config with no effect, and so does this repository at its own root.

no-floating-promises could not run even if the file were read: it is type-aware, oxlint reads types only through oxlint-tsgolint, and neither that package nor --type-aware was present. $schema pointed at a Rust source file rather than a JSON schema.

What is the new behavior?

  • oxlint.json becomes .oxlintrc.json, in the two application templates and at this repository's root.
  • $schema becomes ./node_modules/oxlint/configuration_schema.json, and rule names take oxlint's own typescript/ prefix. Both match nestjs/nest's own .oxlintrc.json.
  • oxlint-tsgolint is added to the templates' devDependencies and lint becomes oxlint --type-aware src/ test/, which is what makes no-floating-promises actually run.
  • That rule moves from warn to error. oxlint exits 0 on warnings, so at warn it would report an unawaited bootstrap() and still leave the command green.

The build is untouched: copy:lib already copies dotfiles under files/, which is how .prettierrc reaches generated projects today. Confirmed in dist after npm run build.

If you would rather not add a dependency, the other coherent option is to drop no-floating-promises from the template, which is what nestjs/nest does for itself. I kept it because it is the only mistake the scaffold currently claims to catch, but say the word and I will cut that half; the rename stands on its own either way.

Does this PR introduce a breaking change?

  • Yes
  • No

Existing projects are untouched. A project generated after this change gets a config that works, where before it got one that did nothing.

Other information

Verified on oxlint 1.80.0 against a project generated from this branch, with an unawaited bootstrap() in main.ts:

setup npm run lint
master today exit 0, silent
config renamed, nothing else exit 0, silent
this branch exit 1, src/main.ts:8:1 error typescript(no-floating-promises)

The middle row is why both halves are here; neither alone changes the outcome.

npm run lint, npm run typecheck and npm test all pass on this branch, 730 tests in 42 files. Node 24.18.0, macOS.

oxlint auto-discovers .oxlintrc.json, .oxlintrc.jsonc, oxlint.config.ts and oxlint.config.mts.
It does not look for oxlint.json, so the file this repository ships, both at its own root and
into every generated project, is never read: a rule added to it changes nothing and reports no
error. Renaming it is the whole fix, and nestjs/nest already uses .oxlintrc.json. The build needs
no change, since cpx already copies dotfiles under files/ for .prettierrc.

no-floating-promises needs a second fix on top of that. It is a type-aware rule, oxlint reads
types only through oxlint-tsgolint, and neither that package nor --type-aware was present, so the
rule was skipped in silence. Both are added here, and the level moves from warn to error, because
oxlint exits 0 on warnings and a warn-level rule cannot fail the script it is declared in.

$schema pointed at crates/oxc_linter/src/rules.rs, a Rust source file rather than a JSON schema,
and now points at ./node_modules/oxlint/configuration_schema.json, which is what the oxlint
documentation and nestjs/nest both use. Rule names take oxlint's own typescript/ prefix with it.

Verified against a project generated from this branch, with an unawaited bootstrap() in main.ts:
npm run lint exits 0 and prints nothing on master, exits 0 with the config renamed but no type
information available, and exits 1 naming main.ts:8 once both halves are in place.
Raising no-floating-promises to error is only honest if the code this schematic
writes can pass it, and the CommonJS main.ts could not: it ends in a bare
bootstrap(), which is the exact call the rule exists to catch. Generated as it
stood, npm run lint failed on an untouched project.

void is the fix rather than .catch(console.error), and the difference matters.
void changes nothing at runtime; it is the acknowledgement the rule asks for, and
a rejected bootstrap still takes the process down. A catch handler would swallow
that and leave a half-started application running, which is the objection raised
against exactly that suggestion in nestjs/nest#14965.

The ESM template already passes, because top-level await let it write
await bootstrap(). This gives the CommonJS one the same property by the only
means it has.

Verified by generating both, installing, and linting: cjs and esm each exit 0
with nothing reported.
@mhalikosen

Copy link
Copy Markdown
Author

Pushed a second commit, because the first one had a hole I only found afterwards.

Raising no-floating-promises to error is only honest if the code this schematic writes can pass it, and the CommonJS main.ts could not. It ends in a bare bootstrap();, which is the exact call the rule exists to catch, so a freshly generated CommonJS project failed npm run lint on its own untouched output. My earlier verification used the ESM template, which passes because top-level await let it write await bootstrap();.

The CommonJS template now writes void bootstrap();.

void rather than .catch(console.error) on purpose. void changes nothing at runtime: it is the acknowledgement the rule asks for in its own help text, and a rejected bootstrap still takes the process down. A catch handler would swallow that and leave a half-started application running, which is the objection @micalevisk raised against that same suggestion in nestjs/nest#14965.

That issue is worth reading next to this PR, since it is the same rule and the same line. In April 2025 someone reported that nest new generated a main.ts which tripped @typescript-eslint/no-floating-promises, and the answer was "That's why it's set to warn". Under ESLint that was a real, visible warning and warn was a deliberate call about it. Ported to oxlint, the same two lines produce nothing at all: the file is not read, and the rule cannot run. So warn today is not the same decision it was then, and the line the decision was about is still in the CommonJS template.

Both templates now generate, install and lint clean:

template last line of main.ts npm run lint
cjs void bootstrap(); exit 0, nothing reported
esm await bootstrap(); exit 0, nothing reported

npm run lint, npm run typecheck and the full suite still pass, 730 tests in 42 files.

If you would rather leave main.ts alone, the alternative is to keep the rule at warn; it would then run and print, without failing the command. I still prefer error, because a warn that can never fail the script it is declared in is close to the state this PR is about. Either way the rename half stands on its own.

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