Skip to content

fix: keep multi-line interpolations from reporting false errors - #36

Merged
quantizor merged 8 commits into
mainfrom
fix/multiline-url-placeholder
Sep 28, 2026
Merged

quantizor merged 8 commits into
mainfrom
fix/multiline-url-placeholder

Conversation

@quantizor

@quantizor quantizor commented Sep 27, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes the remaining shapes from #13: an interpolation written across several lines is now checked the same as when written on one line. Prettier produces these layouts often, for example when it wraps ${({ theme }) => theme.breakpoints.md} inside @media, or url(${props => props.image}), onto several lines.

What was wrong

Before the CSS language service reads a template, the plugin swaps each ${...} for filler of the same length. The filler used to keep the interpolation's own line breaks. At runtime the interpolated value contains none of those source line breaks, and in several places a line break changes how CSS reads the text:

  • Inside a quoted string, an unquoted url() argument, or a // comment, a raw line break ends the token early. The string's real closing quote then opened a new string, which led to errors like ) expected, or silently stopped checking for the rest of the template.
  • In a property name or an at-rule prelude such as @media, @keyframes, @container, or @layer, a line break split one name into several tokens and produced errors like colon expected or { expected.

What changed

  • No filler keeps an interpolation's line breaks. Each is written over like the rest of the interpolation.
  • An interpolation that starts inside a comment, a quoted string, or an unquoted url() argument is filled with plain x over its whole length. This holds both in the text the plugin uses to classify later interpolations and in the text the CSS language service checks, so a mixin later on the same line is still read as a mixin.
  • Diagnostic, hover, folding, and completion positions are unaffected, since position mapping reads the original template's line starts.
  • An interpolation inside url() followed by more text, such as url(#${id}-grad), no longer reports false errors, even on one line.
  • Emmet reads the caret's line straight from the text, so it now gets a document of the same length whose lines are the template's lines. This also brings Emmet suggestions back on a line that follows a lone carriage return, a line continuation, or a line separator inside a string.
  • Substitution builds its analysis text once. It is faster than before on ordinary templates, since per-character fill loops are gone.

Verification

  • Language-service tests cover each shape, and every case includes a positive-control typo that must still be reported:

    • issue Semi-colon expected after Prettier format #13's snippet
    • quoted and unquoted url() and url(#${id})
    • strings
    • several interpolations on one line
    • // and /* */ comments
    • @media, @keyframes, @container, and @layer preludes
    • property names and custom property names
    • Emmet on lines after a multi-line interpolation

    Rows labeled Semi-colon expected after Prettier format #13 fail on main and pass here; the rest are regression guards.

  • Two fuzz comparisons, each over roughly 19,000 generated templates, check that multi-line diagnostics now match the single-line version.

  • New scaling-guardrail shapes cover many multi-line interpolations alternating between strings and code, and completions on a large template with many multi-line interpolations.

  • A unit test checks that Emmet's line view has the same length and offsets as the virtual document, and that its lines agree with its text.

  • yarn verify passes.

A separate commit rewords a pending changeset. On TypeScript older than 5.0, 1.0.1 left an unexplained activation error in the TypeScript server log. This release logs that TypeScript 5.0 or newer is required.

@changeset-bot

changeset-bot Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: ef7bd42

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

This PR includes changesets to release 1 package
Name Type
@styled/typescript-styled-plugin Minor

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

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The substitution fix itself looks right: I could not make it report a false error, and the Emmet view returns the same suggestions as the single-line shape in every layout I tried. One new object is internally inconsistent, though: the Emmet document shim hands a third-party library a TextDocument whose text does not match its own offsets, and the spec sentence that licenses it is factually wrong. Worth fixing in this PR rather than leaving as a trap for the next reader.

Reviewed changes — every fill now writes over the whole placeholder, plus the Emmet line view, the spec, and the tests that cover them.

  • Solid fill for opaque positions — a placeholder that starts inside a /* */ or // comment, a string, or an unquoted url() argument fills with x over its whole length, so the token the CSS service reads no longer ends early. The decision comes from one walk of the spans against the run list (findSolidSpans), and a backslash-started run is excluded so an escape that swallows the placeholder's first character keeps its branch fill.
  • No fill keeps a line terminator — fillPlaceholder, wrapPlaceholder, and maskSubstitutions became String.repeat, the per-character loops are gone, and the hex fill's special case collapses into wrapPlaceholder(placeholderText, { open: HEX_FILL }). The masked text is built once (buildSyntaxText), so the runs are found against the final text.
  • Emmet line view — withTemplateLineBreaks (src/features/completions.ts:360) gives Emmet a copy of the virtual document with \n written at the end of every template line, which restores Emmet after a multi-line fill, a lone \r, a line continuation, or a U+2028/U+2029 kept inside a string.
  • Spec and changesets — docs/architecture.md gains the solid-fill invariant and the Emmet line view, and the masked-text and cost bullets are reworded for the new fill; the multi-line and Emmet changesets are added and the TypeScript-floor changeset is reworded.
  • Tests and guardrails — diagnostics rows for each fixed shape (every one keeps a colr positive control), substitution rows per fill, an Emmet placement table, four compare-release cases, and two scaling-guardrail shapes that alternate solid and classified placeholders.

What I ran: corepack yarn test:unit and corepack yarn test:scaling are green on the head. Checking out the pre-PR src/template/template-substitutions.ts and src/features/completions.ts against the new tests fails exactly the 53 rows this PR adds or rewrites and nothing else, so the helper rewrite does not move any single-line behavior. A 75-case differential (46 shapes I wrote, across styled, css, and keyframes tags, including CRLF, U+2028/U+2029, an emoji, unterminated comments and strings, and every at-rule and selector position) shows the multi-line variant never reports a diagnostic the single-line variant does not.

ℹ️ Nitpicks

  • getSubstitution still calls fillPlaceholder(placeholderText, 'x') in three branches, which is now character-for-character SOLID_FILL.repeat(placeholderText.length). Using the constant the PR introduced keeps one name for "the fill character" and saves a reader from wondering whether the two spellings mean the same thing.
  • The new per-request work in the completions path has no scaling-guardrail case: every entry in completionCases ends in a line that already has its \n, so the string-building branch of withTemplateLineBreaks is never timed. One large template with a multi-line interpolation in the middle would cover it (AGENTS.md, "Performance").

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using Space Bunny (free) | 𝕏

Comment thread src/features/completions.ts Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No blocking issues — the same-length view is sound. Two doc-and-test nits inline.

Reviewed changes — what landed since the last Pullfrog review: the Emmet line view rebuilt as a real TextDocument, the solid-fill constant, two new url(#…) diagnostic rows, a new view unit test, and the spec and changeset reword.

  • Emmet line view is a consistent document — withTemplateLineBreaks no longer returns a hand-rolled object mixing aligned with the unaligned document's offsets and lineCount; it writes \n over each template line's last character, keeping the substituted text's length, and returns TextDocument.create(...) over it. docs/architecture.md:86's "same length, positions unchanged" claim is now true, and the new test/unit/completions.test.ts pins length, per-offset agreement, and self-consistent line ranges.
  • Template start computed once — toVirtualDocOffset(0, context) is read into templateStart and shared by the Emmet view and the at-keyword range.
  • SOLID_FILL in the three getSubstitution branches — the literal 'x' is gone, so one name means "the fill character".
  • Two single-line url(#…) rows — fill: url(#${id}-grad) and mask: url(#${p => p.id}_mask) join the colr-control group, and the spec's block-position example is narrowed to the case that still produces $a:0.
  • Scaling guardrail for the completions path — createManyMultilinePlaceholdersTemplate exercises the string-building branch, which every existing completion case skips.
  • Changeset — records that 1.0.1 reported these errors too, and that Emmet suggestions are back after a lone carriage return, a line continuation, or a line separator inside a string.

What I ran: corepack yarn test:unit (928 passed), corepack yarn test:scaling (all ratios inside threshold, the new completions case at 3.29), format:check, lint, and typecheck are clean. Checking out main's template-substitutions.ts and completions.ts against the new tests fails exactly the 58 rows this PR adds or rewrites and moves no pre-existing one. A fuzz over 824 templates (mixed \n / \r\n / lone \r / U+2028 / U+2029, 0–3 placeholders in 14 positions, 10 escape shapes) held both invariants across ~97k positions, and completion entry names match the single-line equivalent for 17 shape pairs.

ℹ️ CRLF is absent from the new table on purpose, and one offset really does differ

The fixture table has no CRLF row, and adding one would fail twice. The line-range assertion is the obvious half (inline); this is the other half. A position whose character falls inside a \r\n pair (the \n) maps one character earlier in the view than in the document, because vscode-languageserver-textdocument's offsetAt runs ensureBeforeEOL and never returns an offset between \r and \n, while the plugin's line map happily does. It needs a \r\n outside every placeholder plus one line that needs realignment, so the fresh view is built at all. It is unreachable in practice: the position the plugin receives comes from a client offset through TypeScript's own line map (relative(baseLC, getLineAndChar(...)) in the decorator), so character never exceeds the line's visible content. Every reachable position agrees, and the view's line starts match the plugin's line map even when a template mixes CRLF with a multi-line placeholder.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using Space Bunny (free) | 𝕏

Comment thread src/features/completions.ts Outdated
Comment thread test/unit/completions.test.ts Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found. Both nits from the last pass are addressed, and nothing in this delta needs a change.

Reviewed changes — what landed since the last Pullfrog review (920f315 → ef7bd42, one commit, two files).

  • The JSDoc now says overwrite, not append — withTemplateLineBreaks documents the \n as "written over the last character of every template line" and adds "An overwrite, not an insert, so the view keeps the document's length and offsets", then names what that character always is: the template's line terminator, a fill character, or a space an escape stand-in wrote. That matches the invariant docs/architecture.md:86 states, so the comment and the spec no longer read as two different mechanisms.
  • The line-range assertion is scoped to a realigned view — a new notStrictEqual(view, document) guard keeps the block on the case the prior comment described, and the comparison drops the trailing \r a surviving \r\n leaves on a split('\n') line, which is the one difference getLineRange and Emmet's getCurrentLine legitimately have.

What I ran: corepack yarn test:unit (928 passed, including the three withTemplateLineBreaks rows), corepack yarn test:scaling (every ratio inside threshold; the new completions case at 3.65, the two new substitution cases at 4.40 and 3.89), lint at 0 warnings, and typecheck are clean.

I also checked the new assertion is true in general and not only for its three fixtures, since that was the substance of the prior comment. Over 6400 generated templates (24 fragments covering multi-line placeholders, strings, comments, url() and url(#…), line continuations, hex escapes and at-rules, combined one to three deep across \n, \r\n, lone \r, U+2028 and U+2029) and ~219k positions: every realigned view keeps the document's length, its line starts inside the template region are exactly templateStart + lineStarts, split('\n').length === lineCount always holds, and the replace(/\r$/, '') comparison holds on every line. The only disagreements were the already-documented offsetAt case where a position's character falls inside a \r\n pair, one per template ending in \r\n, unchanged from the last pass and unreachable from a client-derived position.

Pullfrog  | View workflow run | Using Space Bunny (free) | 𝕏

@quantizor
quantizor merged commit e13d896 into main Sep 28, 2026
20 of 21 checks passed
@quantizor
quantizor deleted the fix/multiline-url-placeholder branch September 28, 2026 00:28
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