Skip to content

fix(isolated_declarations): drop required type check for private parameter properties on private constructors#21515

Merged
graphite-app[bot] merged 1 commit intomainfrom
fix/isolated-declarations-private-ctor-inferred-param-type
Apr 20, 2026
Merged

fix(isolated_declarations): drop required type check for private parameter properties on private constructors#21515
graphite-app[bot] merged 1 commit intomainfrom
fix/isolated-declarations-private-ctor-inferred-param-type

Conversation

@Dunqing
Copy link
Copy Markdown
Member

@Dunqing Dunqing commented Apr 17, 2026

Summary

A private parameter property on a private constructor doesn't need an explicit type annotation:

  • The constructor signature is collapsed to private constructor(); — the parameter is never emitted.
  • The parameter-property class member is emitted as private readonly name; — type-erased.

Since the parameter's type does not appear in the .d.ts output in either place, no type is needed. This matches tsc --isolatedDeclarations. Non-private parameter properties on a private constructor still require a type, because the class member keeps the type.

Example

export class GreyhoundServer {
  private constructor(
    private readonly logger = new ConsoleLogger(['greyhound-server']),
  ) {}
}

Before: TS9011: Parameter must have an explicit type annotation.
After: no error — emits private readonly logger; and private constructor();, matching tsc.

Test plan

  • Added snapshot fixtures covering both the no-error case (private param property) and the still-errors case (public param property on private constructor)
  • cargo test -p oxc_isolated_declarations

Closes #21503 (case 1)

Copy link
Copy Markdown
Member Author

Dunqing commented Apr 17, 2026


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent changes, fast-track this PR to the front of the merge queue

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions github-actions Bot added A-isolated-declarations Isolated Declarations C-bug Category - Bug labels Apr 17, 2026
@Dunqing Dunqing changed the title fix(linter/unicorn): handle computed property access in prefer-dom-node-remove rule (#21470) fix(isolated_declarations): drop required type check for private parameter properties on private constructors Apr 17, 2026
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 17, 2026

Merging this PR will not alter performance

✅ 44 untouched benchmarks
⏩ 7 skipped benchmarks1


Comparing fix/isolated-declarations-private-ctor-inferred-param-type (ee38f92) with main (065ce47)

Open in CodSpeed

Footnotes

  1. 7 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@Dunqing Dunqing force-pushed the fix/isolated-declarations-private-ctor-inferred-param-type branch 3 times, most recently from 44bf75d to aed63e8 Compare April 17, 2026 06:19
@Dunqing Dunqing marked this pull request as ready for review April 17, 2026 06:43
@Dunqing Dunqing added the 0-merge Merge with Graphite Merge Queue label Apr 20, 2026
Copy link
Copy Markdown
Member Author

Dunqing commented Apr 20, 2026

Merge activity

  • Apr 20, 4:37 AM UTC: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Apr 20, 4:37 AM UTC: Dunqing added this pull request to the Graphite merge queue.
  • Apr 20, 5:06 AM UTC: The Graphite merge queue couldn't merge this PR because it had merge conflicts.
  • Apr 20, 8:11 AM UTC: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Apr 20, 8:24 AM UTC: Dunqing added this pull request to the Graphite merge queue.
  • Apr 20, 8:42 AM UTC: Merged by the Graphite merge queue.

@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label Apr 20, 2026
@Dunqing Dunqing force-pushed the fix/isolated-declarations-private-ctor-inferred-param-type branch from aed63e8 to ee38f92 Compare April 20, 2026 05:25
@Dunqing Dunqing added the 0-merge Merge with Graphite Merge Queue label Apr 20, 2026
@Dunqing Dunqing force-pushed the fix/isolated-declarations-private-ctor-inferred-param-type branch from ee38f92 to 33b0609 Compare April 20, 2026 08:11
…meter properties on private constructors (#21515)

## Summary

A private parameter property on a private constructor doesn't need an explicit type annotation:

- The constructor signature is collapsed to `private constructor();` — the parameter is never emitted.
- The parameter-property class member is emitted as `private readonly name;` — type-erased.

Since the parameter's type does not appear in the `.d.ts` output in either place, no type is needed. This matches `tsc --isolatedDeclarations`. Non-private parameter properties on a private constructor still require a type, because the class member keeps the type.

## Example

```ts
export class GreyhoundServer {
  private constructor(
    private readonly logger = new ConsoleLogger(['greyhound-server']),
  ) {}
}
```

Before: `TS9011: Parameter must have an explicit type annotation`.
After: no error — emits `private readonly logger;` and `private constructor();`, matching tsc.

## Test plan

- [x] Added snapshot fixtures covering both the no-error case (private param property) and the still-errors case (public param property on private constructor)
- [x] `cargo test -p oxc_isolated_declarations`

Closes #21503 (case 1)
@graphite-app graphite-app Bot force-pushed the fix/isolated-declarations-private-ctor-inferred-param-type branch from 33b0609 to 48967e8 Compare April 20, 2026 08:33
@graphite-app graphite-app Bot merged commit 48967e8 into main Apr 20, 2026
27 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label Apr 20, 2026
@graphite-app graphite-app Bot deleted the fix/isolated-declarations-private-ctor-inferred-param-type branch April 20, 2026 08:42
camc314 added a commit that referenced this pull request Apr 20, 2026
### 🐛 Bug Fixes

- 48967e8 isolated_declarations: Drop required type check for private
parameter properties on private constructors (#21515) (Dunqing)
- 91e5bde transformer/typescript: Preserve computed-key static block
when class has an empty constructor (#21562) (Dunqing)
- 50e9d26 mangler: Assign correct slot to shadowed function-expression
names (#21535) (Dunqing)
- 065ce47 isolated_declarations: Collect types from private accessors
for paired inference (#21516) (Dunqing)
- 00fc136 codegen: Preserve coverage comments before object properties
(#21312) (bab)
- d676e0c minifier: Mark LHS of `??=` as read when converting from `==
null &&` (#21546) (Gunnlaugur Thor Briem)

### ⚡ Performance

- e45efc5 parser: Reduce `try_parse` usage in favour of `lookahead`
(#21532) (Boshen)
- ddb1bf8 parser: Avoid redundant `IdentifierReference` clone in
shorthand property (#21511) (Boshen)
- be2b392 allocator: Store pointers directly in `Arena` (#21483)
(overlookmotel)

Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
Co-authored-by: Cameron <cameron.clark@hey.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-isolated-declarations Isolated Declarations C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

isolatedDeclarations: oxc is stricter than tsc for inferred accessor return types and constructor parameter defaults

1 participant