feat(transformer/legacy-decorator): add strictNullChecks option for nullable-union design:type#22266
Conversation
3142b27 to
8d25280
Compare
8d25280 to
200d70c
Compare
|
It looks like we should add an option to turn off |
200d70c to
4c078f6
Compare
… nullable-union design:type ## Summary Adds a `strictNullChecks` field to `DecoratorOptions` (default `true`) that controls whether `null` and `undefined` are elided from union design:type metadata. The default preserves tsc strict semantics (`T | null` → `Object`); setting `strictNullChecks: false` matches babel-plugin-transform-typescript-metadata and tsc with `--strictNullChecks=false`, emitting the underlying primitive (`T | null` → `String` when `T = string`). ## Motivation When `strictNullChecks` is off, tsc emits the constructor of the underlying primitive. babel-plugin-transform-typescript-metadata always elides null and undefined regardless of tsconfig. Downstream consumers (NestJS Swagger, class-validator, TypeORM, AutoMapper) treat `Object` as "no metadata" and silently drop fields, so users with `strictNullChecks: false` (common in older codebases) get worse behaviour under OXC than under tsc/babel. This PR adds the option without changing the default, matching Dunqing's review comment on oxc-project#22266. ## Implementation ```rust pub struct DecoratorOptions { pub legacy: bool, pub emit_decorator_metadata: bool, pub strict_null_checks: bool, // default true } ``` Threaded through `LegacyDecorator::new` → `LegacyDecoratorMetadata::new` → gated in `serialize_union_or_intersection_constituents`: ```rust TSType::TSNullKeyword(_) | TSType::TSUndefinedKeyword(_) if !is_intersection && !self.strict_null_checks => continue, ``` Mirrored at the napi binding (`napi/transform/src/transformer.rs`) so direct Node consumers can flip it the day this lands. Consumers that wire tsconfig (rolldown, bundlers) can forward `compilerOptions.strictNullChecks` to it. ## Test plan - [x] 7 unit tests in `tests/integrations/decorator_metadata.rs` covering `T | null` under both `strictNullChecks: true` (emits Object, current behaviour) and `strictNullChecks: false` (emits primitive), plus `T | undefined`, `T | null | undefined`, the `null | undefined` void-only case, the `string | number` distinct-primitive case (still Object), and the intersection-with-null regression - [x] New conformance fixture `oxc/metadata/nullable-union/` with its own `options.json` setting `strictNullChecks: false` - [x] `cargo test -p oxc_transformer` passes - [x] `cargo run -p oxc_transform_conformance` shows 0 regressions; baseline fixtures still pass plus the new `nullable-union` - [x] `cargo fmt` and `cargo clippy --tests --no-deps` clean across the transformer crate and the napi binding AI assistance was used in writing this patch and tests; the contributor has reviewed and tested locally.
strictNullChecks option for nullable-union design:type
Merging this PR will not alter performance
Comparing Footnotes
|
… nullable-union design:type ## Summary Adds a `strictNullChecks` field to `DecoratorOptions` (default `true`) that controls whether `null` and `undefined` are elided from union design:type metadata. The default preserves tsc strict semantics (`T | null` → `Object`); setting `strictNullChecks: false` matches babel-plugin-transform-typescript-metadata and tsc with `--strictNullChecks=false`, emitting the underlying primitive (`T | null` → `String` when `T = string`). ## Motivation When `strictNullChecks` is off, tsc emits the constructor of the underlying primitive. babel-plugin-transform-typescript-metadata always elides null and undefined regardless of tsconfig. Downstream consumers (NestJS Swagger, class-validator, TypeORM, AutoMapper) treat `Object` as "no metadata" and silently drop fields, so users with `strictNullChecks: false` (common in older codebases) get worse behaviour under OXC than under tsc/babel. This PR adds the option without changing the default, matching Dunqing's review comment on oxc-project#22266. ## Implementation ```rust pub struct DecoratorOptions { pub legacy: bool, pub emit_decorator_metadata: bool, pub strict_null_checks: bool, // default true } ``` Threaded through `LegacyDecorator::new` → `LegacyDecoratorMetadata::new` → gated in `serialize_union_or_intersection_constituents`: ```rust TSType::TSNullKeyword(_) | TSType::TSUndefinedKeyword(_) if !is_intersection && !self.strict_null_checks => continue, ``` Mirrored at the napi binding (`napi/transform/src/transformer.rs`) so direct Node consumers can flip it the day this lands. Consumers that wire tsconfig (rolldown, bundlers) can forward `compilerOptions.strictNullChecks` to it. ## Test plan - [x] 7 unit tests in `tests/integrations/decorator_metadata.rs` covering `T | null` under both `strictNullChecks: true` (emits Object, current behaviour) and `strictNullChecks: false` (emits primitive), plus `T | undefined`, `T | null | undefined`, the `null | undefined` void-only case, the `string | number` distinct-primitive case (still Object), and the intersection-with-null regression - [x] New conformance fixture `oxc/metadata/nullable-union/` with its own `options.json` setting `strictNullChecks: false` - [x] `cargo test -p oxc_transformer` passes - [x] `cargo run -p oxc_transform_conformance` shows 0 regressions; baseline fixtures still pass plus the new `nullable-union` - [x] `cargo fmt` and `cargo clippy --tests --no-deps` clean across the transformer crate and the napi binding AI assistance was used in writing this patch and tests; the contributor has reviewed and tested locally.
4c078f6 to
738ee7b
Compare
|
@Dunqing i pushed a commit that allows for turning it on and off. Let me know if this unblocks the issue! |
… nullable-union design:type ## Summary Adds a `strictNullChecks` field to `DecoratorOptions` (default `true`) that controls whether `null` and `undefined` are elided from union design:type metadata. The default preserves tsc strict semantics (`T | null` → `Object`); setting `strictNullChecks: false` matches babel-plugin-transform-typescript-metadata and tsc with `--strictNullChecks=false`, emitting the underlying primitive (`T | null` → `String` when `T = string`). ## Motivation When `strictNullChecks` is off, tsc emits the constructor of the underlying primitive. babel-plugin-transform-typescript-metadata always elides null and undefined regardless of tsconfig. Downstream consumers (NestJS Swagger, class-validator, TypeORM, AutoMapper) treat `Object` as "no metadata" and silently drop fields, so users with `strictNullChecks: false` (common in older codebases) get worse behaviour under OXC than under tsc/babel. This PR adds the option without changing the default, matching Dunqing's review comment on oxc-project#22266. ## Implementation ```rust pub struct DecoratorOptions { pub legacy: bool, pub emit_decorator_metadata: bool, pub strict_null_checks: bool, // default true } ``` Threaded through `LegacyDecorator::new` → `LegacyDecoratorMetadata::new` → gated in `serialize_union_or_intersection_constituents`: ```rust TSType::TSNullKeyword(_) | TSType::TSUndefinedKeyword(_) if !is_intersection && !self.strict_null_checks => continue, ``` Mirrored at the napi binding (`napi/transform/src/transformer.rs`) so direct Node consumers can flip it the day this lands. Consumers that wire tsconfig (rolldown, bundlers) can forward `compilerOptions.strictNullChecks` to it. ## Test plan - [x] 7 unit tests in `tests/integrations/decorator_metadata.rs` covering `T | null` under both `strictNullChecks: true` (emits Object, current behaviour) and `strictNullChecks: false` (emits primitive), plus `T | undefined`, `T | null | undefined`, the `null | undefined` void-only case, the `string | number` distinct-primitive case (still Object), and the intersection-with-null regression - [x] New conformance fixture `oxc/metadata/nullable-union/` with its own `options.json` setting `strictNullChecks: false` - [x] `cargo test -p oxc_transformer` passes - [x] `cargo run -p oxc_transform_conformance` shows 0 regressions; baseline fixtures still pass plus the new `nullable-union` - [x] `cargo fmt` and `cargo clippy --tests --no-deps` clean across the transformer crate and the napi binding AI assistance was used in writing this patch and tests; the contributor has reviewed and tested locally.
bd7da79 to
caee2da
Compare
…e fixture Removed the integration-test file at `crates/oxc_transformer/tests/integrations/decorator_metadata.rs` in favour of extending the existing `tasks/transform_conformance/tests/legacy-decorators/test/fixtures/oxc/metadata/nullable-union/` fixture. Per project convention, oxc_transformer's legacy-decorator metadata emission is exercised through conformance fixtures rather than per-crate unit tests. The fixture now covers, under `strictNullChecks: false`: - `string | null` → String - `number | undefined` → Number - `boolean | null | undefined` → Boolean - `null | undefined` → void 0 - `string | number` → Object
caee2da to
6fc9dc2
Compare
Adds two cases in transform.test.ts under `legacy decorator > strictNullChecks`: - default (strictNullChecks: true) emits Object for `string | null` - strictNullChecks: false elides null and emits the underlying String constructor
### 🚀 Features - e857b0c napi/minify: Expose legalComments option and result (#20370) (Boshen) - 661132d parser: More friendly error messages for rest assignment target and rest binding element (#22719) (sapphi-red) - ee659b6 transformer/legacy-decorator: Add `strictNullChecks` option for nullable-union design:type (#22266) (Kyle Cannon) ### 🐛 Bug Fixes - e1d064e transformer/class-properties: Reparent lifted private method helpers (#22716) (Cameron) - 4ac0fca minifier: Preserve `0 && (module.exports = { ... })` cjs-module-lexer hint (#22729) (Dunqing) - 40ff611 minifier: Mark peephole loop changed when dropping dead-after-throw statement (#22722) (Dunqing) - 2f7b210 codegen: Emit pife-arrow/function leading comments inside the wrap (#22720) (Dunqing) - e184f74 parser: Improve invalid `import` property access diagnostic (#22693) (camc314) - 7baed9c transformer/private-method: Clear inherited strict flags (#22508) (camc314) - a9ad27e parser: Keep annotation comments leading without preceding newline (#22711) (Dunqing) - 9ea4d64 minifier: Re-evaluate pure/no-side-effects flags after peephole inlining (#22595) (Dunqing) - 07afbb6 minifier: Drop empty-body IIFE wrapper when called with arguments (#22589) (Dunqing) - fa7c463 semantic: Correct TS enum member symbol spans (#22689) (camc314) - 26b9396 semantic: Resolve parameter decorators outside parameter scope (#22623) (camc314) - b284045 parser: Switch to module goal eagerly on `export` (#22684) (Boshen) - dfa931d semantic: Propagate unresolved auto-increment enum value instead of defaulting to 0 (#22646) (Dunqing) - 69a6ba6 transformer/legacy-decorator: Emit Array for ReadonlyArray<T> in decorator metadata (#22265) (Kyle Cannon) - e421ef0 transformer/legacy-decorator: Return runtime binding for design:type (#22640) (Dunqing) - d61e1d7 codegen: Preserve verbatim text of pure/no-side-effects comments (#22525) (Dunqing) - 702b14e minifier: Preserve IIFE structure in DCE-only mode (#22547) (Dunqing) - 917da24 parser: Apply PURE comment through member-access chains (#22566) (Dunqing) - a069b1c codegen: Preserve quotes for cjs-module-lexer equality strings (#22551) (Dunqing) ### ⚡ Performance - 2f623b0 semantic: Skip unresolved checks for re-exports (#22660) (camc314) - 0d9553d semantic: Early-exit `check_object_expression` for objects with <2 properties (#22668) (Dunqing) - d721ad9 semantic: Use direct grandparent lookup for TS type parameters (#22658) (camc314) - 0aff288 semantic: Reorder numeric literal strict mode checks (#22657) (camc314) - 4d5ddb1 semantic: Reorder binding identifier checks (#22656) (camc314) - e32acd8 semantic: Reorder identifier ambient binding check (#22653) (camc314) - 09fe178 semantic: Reorder ident reference strict mode check (#22652) (camc314) - 4b6add2 semantic: Avoid duplicate ident clone for bindings (#22663) (camc314) - 82f9662 parser: Check identifier kind before context flag (#22662) (camc314) - d7cd951 parser: Fast path identifier parsing and inline operator helpers (#22650) (Boshen) - 7b84314 semantic: Use direct byte access for numeric leading-zero check (#22642) (camc314) - 0345a31 semantic: Pre-size class elements hash map (#22618) (camc314) - 04d3065 minifier: Drop per-call buffers in try_fold_concat (#22596) (Dunqing) - 4f289f1 semantic: Resolve_references_for_current_scope without a temp Vec (#22599) (Dunqing) - e862c15 semantic: Avoid heap alloc for var hoist scope ids (#22603) (Dunqing) - 8ff8674 semantic: Early return if `excess` is `0` in `Stats::increase_by` (#22616) (camc314) - 7a4120e semantic: Pre-reserve unresolved_references using Stats::references (#22580) (Dunqing) Co-authored-by: Dunqing <29533304+Dunqing@users.noreply.github.com>
## Summary Builds on #9563, which upgraded oxc to `0.133.0` and surfaced the `strictNullChecks` decorator-metadata option (from oxc-project/oxc#22266) in the TypeScript types, the validator, and the generated binding. However, #9563's Rust `From<DecoratorOptions> for oxc::transformer::DecoratorOptions` hardcodes `strict_null_checks: true`, and the napi options normalizer doesn't carry the field, so `transform.decorator.strictNullChecks: false` is currently **silently ignored** through rolldown's transform/bundle pipeline (the option exists in the type surface but does nothing). This PR wires it through so the option actually takes effect. ## What changed - `rolldown_common::DecoratorOptions` gains a `strict_null_checks: Option<bool>` field; its `From` impl now uses `options.strict_null_checks.unwrap_or(true)` (mirroring oxc's own default) instead of the hardcoded `true`. - `normalize_binding_transform_options` forwards `strict_null_checks` from the incoming napi options into `DecoratorOptions` (previously dropped). - Docs (`transform.md`) + tests. ## Behavior When `emitDecoratorMetadata` is enabled (legacy decorators): | Source | `strictNullChecks: true` (default) | `strictNullChecks: false` | | --- | --- | --- | | `string \| null` | `Object` (matches `tsc` strict) | `String` (matches `tsc --strictNullChecks false` / `babel-plugin-transform-typescript-metadata`) | | `number \| undefined` | `Object` | `Number` | Defaults to `true`, so there is **no behavior change for existing users**; only an explicit `strictNullChecks: false` is newly honored. > Note: this is **not** inferred from `tsconfig.json` *yet*. `oxc_resolver`'s `CompilerOptions` does not currently parse `strictNullChecks`, so (unlike `experimentalDecorators`/`emitDecoratorMetadata`) it must be set explicitly on `transform.decorator`. Documented in `transform.md`. See Follow-up below. ## Follow-up: inferring from `tsconfig.json` [oxc-project/oxc-resolver#1166](oxc-project/oxc-resolver#1166) (`feat(tsconfig): parse strict and strictNullChecks compiler options`) is the enabling resolver-side change. It adds `strict`/`strictNullChecks` to `CompilerOptions`, inherits them through `extends`, and exposes `effective_strict_null_checks()` implementing tsc's `strictNullChecks ?? strict` precedence. Once it lands and is released, a small downstream follow-up here can forward `compiler_options.effective_strict_null_checks()` in `merge_transform_options_with_tsconfig` into `DecoratorOptions.strict_null_checks` (mapping `None` to the transformer default), plus add the field to the inline `BindingTsconfigCompilerOptions` path, making `strictNullChecks` auto-inferred from tsconfig like the other decorator options. This PR is intentionally scoped to the explicit option so it's useful immediately and independent of that release. ## Test plan - [x] Rebased/merged on top of the latest `main` (which already has oxc `0.133.0`); `cargo check --workspace` passes - [x] New `transform.test.ts` cases assert `string | null` emits `Object` by default and `String` under `strictNullChecks: false` (verified locally; the full suite runs in CI) ## AI usage disclosure Per the [AI Usage Policy](https://rolldown.rs/contribution-guide#ai-usage-policy): AI assistance (Claude Code) was used to implement and verify this change. I have reviewed and tested it locally. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: shulaoda <165626830+shulaoda@users.noreply.github.com>
Summary
Adds a
strictNullChecksfield toDecoratorOptions(defaulttrue) thatcontrols whether
nullandundefinedare elided from uniondesign:typemetadata. The default preserves tsc strict semantics (
T | null→Object);setting
strictNullChecks: falsematches babel-plugin-transform-typescript-metadataand tsc with
--strictNullChecks=false, emitting the underlying primitive(
T | null→StringwhenT = string).Originally this PR unconditionally elided null/undefined. Per @Dunqing's
review, it's now opt-in so the default-strict behaviour is preserved.
Motivation
When
strictNullChecksis off, tsc emits the constructor of the underlyingprimitive. babel-plugin-transform-typescript-metadata always elides null and
undefined regardless of tsconfig. Downstream consumers (NestJS Swagger,
class-validator, TypeORM, AutoMapper) treat
Objectas "no metadata" andsilently drop fields, so users with
strictNullChecks: false(common inolder codebases) get worse behaviour under OXC than under tsc/babel.
Implementation
Threaded through
Decorator::new(options) → LegacyDecorator::new(&options) → LegacyDecoratorMetadata::new(&options)(matching the establishedTypeScript::new(&TypeScriptOptions)precedent), and gated inserialize_union_or_intersection_constituents:Mirrored at the napi binding (
napi/transform/src/transformer.rs) so directNode consumers can flip it the day this lands. Consumers that wire tsconfig
(rolldown, bundlers) can forward
compilerOptions.strictNullChecksto it.Behaviour matrix
strictNullChecks: true(default)strictNullChecks: falsestring | nullObject(matches tsc strict, current behaviour)String(matches babel, tsc non-strict)number | undefinedObjectNumberboolean | null | undefinedObjectBooleannull | undefinedvoid 0void 0string | numberObjectObject(distinct primitives)string & nullObjectObject(intersection unchanged)Test plan
tests/integrations/decorator_metadata.rscoveringT | nullunder bothstrictNullChecks: true(emitsObject, currentbehaviour) and
strictNullChecks: false(emits primitive), plusT | undefined,T | null | undefined, thenull | undefinedvoid-onlycase, the
string | numberdistinct-primitive case (stillObject), andthe intersection-with-null regression
oxc/metadata/nullable-union/with its ownoptions.jsonsettingstrictNullChecks: falsecargo test -p oxc_transformerpasses (31 unit + 11 integration)cargo run -p oxc_transform_conformanceshows 0 regressions; baselinefixtures still pass plus the new
nullable-unioncargo fmt -p oxc_transformer -p oxc_transform_napiandcargo clippy --tests --no-depscleanAI assistance was used in writing this patch and tests; the contributor has
reviewed and tested locally.