Skip to content

fix: map the empty type-only import's tokens, and guard comments against token matching - #236

Closed
leonidaz wants to merge 1 commit into
sveltejs:tokens-altfrom
leonidaz:feat/tokens-alt-followups
Closed

leonidaz wants to merge 1 commit into
sveltejs:tokens-altfrom
leonidaz:feat/tokens-alt-followups

Conversation

@leonidaz

Copy link
Copy Markdown
Contributor

Stacked on #235 (base: tokens-alt). Two small things carried over from #213 that the token-matching approach needs.

Empty type-only import. #219 prints import type {} from as a single string, and context.write only maps a write whose trimmed text is exactly one token, so with tokens neither type nor the braces matched. Written one token at a time now. Two test cases: braces of import type /* before */ { /* inside */ } from 'module' and the type keyword of import /* c */ type {} from 'm'.

Guard for comments that look like tokens. The matcher relies on write_comment never handing it a bare bracket, which #235 arranges by writing a one-line comment's delimiters together with its text. Nothing tested that, so I added the cases where it can go wrong: a comment is flushed after enter(node), so a comment whose text is the next node's own opening bracket is the one that can be consumed. Against the pre-#235 comment writer, const o = /* { */ { a: 1 }; loses the mapping of its }; with #235 all of these pass:

  • a comment before an object, an array, an arrow function (/* { */, /* [ */, /* ( */)
  • a multi-line comment whose inner line is a lone ( or {, before an arrow or a block
  • a line comment // ), and a commented-out block containing } lines

Each case checks that every bracket outside comments in the output maps to the matching source bracket.

No changeset: #235's covers it. 522 tests pass, lint clean.

🤖 Generated with Claude Code

@pkg-svelte-dev

pkg-svelte-dev Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Install the latest version of esrap from 2729d07:

pnpm add https://pkg.svelte.dev/esrap/c/2729d072b2afcc75ce648da0af983338e91254fe

Open in pkg.svelte.dev: https://pkg.svelte.dev/repos/esrap/pr/236

Note

This PR is from a fork. A maintainer must approve approve each commit before it can be built and installed.

@changeset-bot

changeset-bot Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 2729d07

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@leonidaz
leonidaz force-pushed the feat/tokens-alt-followups branch from 1028d1b to ef24689 Compare September 26, 2026 03:38
…nst token matching

`import type {} from` was written as one string since sveltejs#219, so with tokens
neither the keyword nor the braces could match; write them one token at a time.

Add tests for the comment handling the token matcher relies on: a comment whose
text is a bracket (`/* ( */`, `// )`, a commented-out block, a comment line
that is a lone `}`) must not be consumed as the next source token, and the
real brackets after it must still map exactly.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@leonidaz
leonidaz force-pushed the feat/tokens-alt-followups branch from ef24689 to 2729d07 Compare September 26, 2026 03:40
@Rich-Harris
Rich-Harris deleted the branch sveltejs:tokens-alt September 26, 2026 13:56
@Rich-Harris

Copy link
Copy Markdown
Member

uhhhhh this auto-closed when i merged tokens-alt

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.

2 participants