Skip to content

fix: stop silently dropping TypeScript markers when printing#135

Merged
manuel3108 merged 5 commits into
sveltejs:mainfrom
MathiasWP:fix/preserve-ts-markers
Jun 27, 2026
Merged

fix: stop silently dropping TypeScript markers when printing#135
manuel3108 merged 5 commits into
sveltejs:mainfrom
MathiasWP:fix/preserve-ts-markers

Conversation

@MathiasWP

Copy link
Copy Markdown
Contributor

Fixes cases where esrap silently dropped TypeScript markers from the output:

  • optional parameters (a?: T)
  • class field ?, !, readonly, declare, override
  • type parameters on classes and methods (class A<T>, m<T>())
  • type-parameter modifiers const / in / out
  • mapped-type readonly / ? modifiers (incl. +/-) and as key-remapping

Several modifiers and type parameters were omitted from the output, changing a
program's type contract while still producing valid-looking code:

- optional parameters (a?: T) lost their ? (Identifier handler) — functions,
  arrows, methods, and call/construct/method signatures
- class fields lost ?, !, readonly, declare, override (PropertyDefinition)
- classes and methods lost their type parameters (class A<T>, m<T>())
- type-parameter modifiers const / in / out were dropped (TSTypeParameter)
- mapped-type readonly/? modifiers (incl. +/-) and 'as' key-remapping were
  dropped (TSMappedType)

Adds test/ts-markers.test.js covering each. No existing snapshots change.
@changeset-bot

changeset-bot Bot commented Jun 3, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: caf4586

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
esrap Patch

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

guyutongxue added a commit to piovium/espolar that referenced this pull request Jun 3, 2026
@manuel3108

Copy link
Copy Markdown
Member

Was about to approve, but found one remaining thing? Why did you not use the existing testing pattern for and created custom tests for that? I think i missed that in the PR i merged a couple of minutes ago as well. LMK if you need help with that.

@jycouet jycouet self-requested a review June 26, 2026 18:24
@manuel3108 manuel3108 merged commit 61c8f1f into sveltejs:main Jun 27, 2026
6 checks passed
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.

3 participants