Modernize the plugin and strengthen language-service compatibility - #33
Conversation
🦋 Changeset detectedLatest commit: 6b6cf02 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
🟡 Changes recommended
One or more issues must be addressed before approval.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Modernizes the TypeScript styled-components plugin with an ESM/TypeScript 6 build, modular language-service implementation, and expanded automated coverage.
Changes:
- Replaces legacy tooling with Yarn 4, tsdown, Vitest, Oxlint, and Oxfmt.
- Refactors configuration, substitutions, virtual documents, CSS features, and tsserver integration.
- Adds unit, E2E, package API, performance, CI, and documentation updates.
File summaries
| File | Description |
|---|---|
| vitest.config.ts | Updated as part of this pull request. |
| tsdown.config.ts | Updated as part of this pull request. |
| tsconfig.json | Updated as part of this pull request. |
| test/unit/tsserver-plugin.test.ts | Updated as part of this pull request. |
| test/unit/template-substitutions.test.ts | Updated as part of this pull request. |
| test/unit/template-language-service.test.ts | Updated as part of this pull request. |
| test/unit/styled-virtual-document-provider.test.ts | Updated as part of this pull request. |
| test/unit/plugin-configuration.test.ts | Updated as part of this pull request. |
| test/performance/template-language-service.bench.ts | Updated as part of this pull request. |
| test/performance/template-language-service-fixture.ts | Updated as part of this pull request. |
| test/package-api/tsconfig.json | Updated as part of this pull request. |
| test/package-api/runtime.ts | Updated as part of this pull request. |
| test/package-api/consumer.ts | Updated as part of this pull request. |
| test/e2e/tsserver-fixture/index.ts | Updated as part of this pull request. |
| test/e2e/tsconfig.json | Updated as part of this pull request. |
| test/e2e/styled-project-fixture/tsconfig.json | Updated as part of this pull request. |
| test/e2e/styled-project-fixture/script-kind-tsx.tsx | Updated as part of this pull request. |
| test/e2e/styled-project-fixture/script-kind-jsx.jsx | Updated as part of this pull request. |
| test/e2e/styled-project-fixture/script-kind-js.js | Updated as part of this pull request. |
| test/e2e/styled-project-fixture/main.ts | Updated as part of this pull request. |
| test/e2e/styled-project-fixture/index.ts | Updated as part of this pull request. |
| test/e2e/styled-project-fixture/.vscode/settings.json | Updated as part of this pull request. |
| test/e2e/scenarios/tsserver-test-helpers.ts | Updated as part of this pull request. |
| test/e2e/scenarios/styled-components-syntax.test.ts | Updated as part of this pull request. |
| test/e2e/scenarios/script-kinds.test.ts | Updated as part of this pull request. |
| test/e2e/scenarios/plugin-lifecycle.test.ts | Updated as part of this pull request. |
| test/e2e/scenarios/outlining-spans.test.ts | Updated as part of this pull request. |
| test/e2e/scenarios/hover.test.ts | Updated as part of this pull request. |
| test/e2e/scenarios/emmet-completions.test.ts | Updated as part of this pull request. |
| test/e2e/scenarios/completions.test.ts | Updated as part of this pull request. |
| test/e2e/scenarios/completion-entry-details.test.ts | Updated as part of this pull request. |
| test/e2e/scenarios/code-fixes.test.ts | Updated as part of this pull request. |
| test/e2e/plugin-missing-project-fixture/tsconfig.json | Updated as part of this pull request. |
| test/e2e/plugin-missing-project-fixture/main.ts | Updated as part of this pull request. |
| test/e2e/plugin-missing-project-fixture/index.ts | Updated as part of this pull request. |
| test/e2e/package.json | Updated as part of this pull request. |
| test/e2e/emmet-disabled-project-fixture/tsconfig.json | Updated as part of this pull request. |
| test/e2e/emmet-disabled-project-fixture/main.ts | Updated as part of this pull request. |
| test/e2e/emmet-disabled-project-fixture/index.ts | Updated as part of this pull request. |
| test/e2e/.gitignore | Updated as part of this pull request. |
| src/virtual-document/virtual-document-session-provider.ts | Updated as part of this pull request. |
| src/virtual-document/styled-virtual-document-provider.ts | Updated as part of this pull request. |
| src/tsserver/tsserver-plugin.ts | Updated as part of this pull request. |
| src/tsserver/tsserver-logger.ts | Updated as part of this pull request. |
| src/tsserver/plugin-identity.ts | Updated as part of this pull request. |
| src/test/substituter.test.ts | Updated as part of this pull request. |
| src/template/template-substitutions.ts | Updated as part of this pull request. |
| src/template-language-service.ts | Updated as part of this pull request. |
| src/index.ts | Updated as part of this pull request. |
| src/features/styles-language-services.ts | Updated as part of this pull request. |
| src/features/hover.ts | Updated as part of this pull request. |
| src/features/folding.ts | Updated as part of this pull request. |
| src/features/diagnostics.ts | Updated as part of this pull request. |
| src/features/css-diagnostic-code.ts | Updated as part of this pull request. |
| src/features/completions.ts | Updated as part of this pull request. |
| src/features/code-actions.ts | Updated as part of this pull request. |
| src/configuration/plugin-configuration.ts | Updated as part of this pull request. |
| src/api.ts | Updated as part of this pull request. |
| src/_virtual-document-provider.ts | Updated as part of this pull request. |
| src/_substituter.ts | Updated as part of this pull request. |
| src/_plugin.ts | Updated as part of this pull request. |
| src/_logger.ts | Updated as part of this pull request. |
| src/_language-service.ts | Updated as part of this pull request. |
| src/_configuration.ts | Updated as part of this pull request. |
| README.md | Updated as part of this pull request. |
| package.json | Updated as part of this pull request. |
| e2e/tests/quickFix.js | Updated as part of this pull request. |
| e2e/tests/outliningSpans.js | Updated as part of this pull request. |
| e2e/tests/errors.js | Updated as part of this pull request. |
| e2e/tests/emmetCompletions.js | Updated as part of this pull request. |
| e2e/tests/completions.js | Updated as part of this pull request. |
| e2e/tests/completionEntryDetails.js | Updated as part of this pull request. |
| e2e/tests/_helpers.js | Updated as part of this pull request. |
| e2e/server-fixture/index.js | Updated as part of this pull request. |
| e2e/project-fixture/tsconfig.json | Updated as part of this pull request. |
| e2e/project-fixture/main.ts | Updated as part of this pull request. |
| e2e/project-fixture/index.ts | Updated as part of this pull request. |
| e2e/project-fixture/.vscode/settings.json | Updated as part of this pull request. |
| e2e/package.json | Updated as part of this pull request. |
| e2e/disabled-emmet-project-fixture/tsconfig.json | Updated as part of this pull request. |
| e2e/disabled-emmet-project-fixture/main.ts | Updated as part of this pull request. |
| e2e/disabled-emmet-project-fixture/index.ts | Updated as part of this pull request. |
| docs/usage.md | Updated as part of this pull request. |
| docs/maintenance.md | Updated as part of this pull request. |
| CHANGELOG.md | Updated as part of this pull request. |
| .yarnrc.yml | Updated as part of this pull request. |
| .vscode/tasks.json | Updated as part of this pull request. |
| .vscode/settings.json | Updated as part of this pull request. |
| .vscode/launch.json | Updated as part of this pull request. |
| .prettierrc.json | Updated as part of this pull request. |
| .oxlintrc.json | Updated as part of this pull request. |
| .oxfmtrc.json | Updated as part of this pull request. |
| .gitignore | Updated as part of this pull request. |
| .github/workflows/ci.yml | Updated as part of this pull request. |
| .github/needs_more_info.yml | Updated as part of this pull request. |
| .eslintrc.js | Updated as part of this pull request. |
| .eslintignore | Updated as part of this pull request. |
| .editorconfig | Updated as part of this pull request. |
Review details
Suppressed comments (4)
src/template/template-substitutions.ts:65
- The
.pattern also matches\r, so a placeholder containing CRLF is converted tox\ninstead of preserving\r\n; the added CRLF tests intest/unit/template-substitutions.test.tswill fail. Match all non-line-break code units instead.
src/template/template-substitutions.ts:71 - This branch has the same CRLF corruption in the mixin path:
\ris replaced withxbefore the newline. Preserve both\rand\nhere as well, otherwise multiline placeholders followed by a semicolon lose their original line endings.
src/virtual-document/styled-virtual-document-provider.ts:14 - This adds a context-dependent overload to the exported
VirtualDocumentProvidermapping contract. Existing providers that implement the old one-argument method can still be passed here, but the new feature code will call them with a context and receive wrapper positions without the new boundary check, so wrapper edits/diagnostics can leak into source offsets. Make the context-aware mapping additive or adapt legacy providers before using it.
test/unit/template-substitutions.test.ts:36 - The new test name contains the typo
proeprty; please correct it toproperty.
- Files reviewed: 92/103 changed files
- Comments generated: 20
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
02e03b1 to
cd5ac16
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
Completion translation and malformed top-level configuration handling contain unresolved runtime bugs.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
src/configuration/plugin-configuration.ts:83
- This method is a runtime tsserver boundary, but a
nullor missingconfigurationvalue causes an immediate property-access exception before the malformed configuration can be ignored. Normalize the top-level value as well as its fields soconfigurePlugincannot break the plugin with valid JSON such asconfiguration: null.
src/features/completions.ts:240 - Completion items are allowed to omit
textEdit; in that case the editor should insert at the requested position. Returning a zero-lengthreplacementSpanat offset 0 instead makes accepting such an item insert at the beginning of the template. OmitreplacementSpanwhen no range is available (and handleInsertReplaceEditseparately if the providers can return it).
- Files reviewed: 97/105 changed files
- Comments generated: 0 new
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Critical CI and supply-chain findings, plus a moderate completion issue, remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
src/features/completions.ts:181
VirtualDocumentProvideris exported as a public API, but its contract does not require a virtual document to end with\n}. Hard-coding that trailer here makes valid completion edits at the end of a custom provider's document get rejected (or allows edits into a different trailer). Derive the template end from theTemplateContext/provider mapping instead of assumingStyledVirtualDocumentProvider's suffix.
- Files reviewed: 97/105 changed files
- Comments generated: 2
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
Package API verification fails on the supported Node 22.12.0 minimum because it runs a TypeScript harness with plain Node.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (3)
package.json:82
- The PR description says the package requires Node.js 24.11 or newer, but this published engine range—and the CI/docs—sets the minimum to 22.12.0. Because this is a release-critical constraint for synchronous
require(ESM), please reconcile the implementation and compatibility notes before publishing rather than advertising conflicting host requirements.
test/unit/template-substitutions.test.ts:14 - The test title uses “a entire”; use “an entire” so the newly added test description is grammatically correct.
test/unit/template-substitutions.test.ts:21 - The test title uses “a entire”; use “an entire” so the newly added test description is grammatically correct.
- Files reviewed: 99/107 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
The VS Code SDK setting is invalid, and code fixes fail for a cursor at a diagnostic’s first character.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
.vscode/settings.json:22
- This is not the VS Code TypeScript SDK setting, so the workspace will continue using VS Code's bundled TypeScript despite the surrounding documentation requiring the workspace SDK. Use the built-in
typescript.tsdksetting instead (the deleted fixture setting used that key as well).
src/features/code-actions.ts:97 - A zero-length code-fix request at the first character of a diagnostic is rejected here because
isAfter(right.end, left.start)treats equality as non-overlap. Editors can request fixes with a cursor range, so placing the cursor on the first character ofboarderyields no rename action even though a cursor one character later works; keep the diagnostic end exclusive, but include equality at its start.
- Files reviewed: 99/107 changed files
- Comments generated: 0 new
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Empty whole-declaration interpolations currently shift all subsequent source mappings, and the new globalStyle integration lacks real-tsserver coverage.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
src/configuration/plugin-configuration.ts:52
- The new default
globalStyletag is only checked as configuration data. The real-tsserver fixture overridestagswithoutglobalStyle, and the styled-components syntax matrix does not exercise it, so the advertised host behavior can regress unnoticed. AddglobalStyleto the fixture configuration and syntax scenario.
CHANGELOG.md:173 - Correct the misspelling in this changelog entry.
- Files reviewed: 99/107 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
The broad modernization and an unresolved E2E completion-response contract issue require human review.
Review details
Suppressed comments (3)
src/api.ts:4
- The public API comment is missing “be”, so it currently reads ungrammatically. Please change it to “allows the language service to be consumed by other libraries.”
test/e2e/tsserver-fixture/index.ts:40 translateCompletionEntryintentionally omitsreplacementSpanwhen an LSP completion has notextEdit(seetest/unit/template-language-service.test.ts:108-117), but this protocol model requires the field for every completion. Any E2E assertion that readsitem.replacementSpanfor such a valid response will be unsound or throw; make the field optional to match tsserver responses.
test/unit/tsserver-message-reader.test.ts:29- This test sends both complete frames in one chunk, so it verifies UTF-8 byte length but not the advertised case where a response is split inside a multibyte character. Split the first frame at a byte within
颜色and assert that no message is emitted until the remaining bytes arrive, so regressions in raw-byte buffering are caught.
- Files reviewed: 99/107 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
The E2E harness has unresolved request-matching and bounded-shutdown issues.
Review details
Suppressed comments (2)
test/e2e/scenarios/tsserver-test-helpers.ts:27
- The harness tracks
request_seqonly to clear the pending set, but this helper still selects the first response for a command by arrival order. Scenarios such as the lifecycle and code-fix tests issue multiple requests of the same command, so an out-of-order tsserver response can be asserted against the wrong request; retain each returned sequence number and select responses byrequest_seq(or expose a request-specific helper) to make the claimed response matching reliable.
test/e2e/tsserver-fixture/index.ts:200 - When any request is still pending,
close()marks the server closed but deliberately does not callshutdown(). If tsserver drops a response or remains alive without answering, thecloseevent can never occur, so the pending-request check below is never reached and the test hangs until Vitest's timeout instead of reporting the incomplete request. Add a bounded close timeout/kill path (or otherwise force an exit and reject with the pending sequence numbers).
- Files reviewed: 100/108 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
src/features/diagnostics.ts must honor validate: false before approval.
Review details
Suppressed comments (1)
src/features/diagnostics.ts:25
validateis normalized and updated by the configuration manager, but this feature never consults it before callingdoValidation. As a result,validate: falsestill produces CSS diagnostics (the newplugin-lifecyclescenario attest/e2e/scenarios/plugin-lifecycle.test.ts:69will fail), and configuration changes cannot disable validation. Pass the configuration manager or a validation predicate into this feature and return an empty list when validation is disabled.
- Files reviewed: 100/108 changed files
- Comments generated: 0 new
- Review effort level: Lite
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No blockers in the release tooling itself — four suggestions inline, all in comments, docs, or dependency config.
Reviewed changes
Reviewed the two commits pushed since the last review (29 files): the changesets setup and release automation, the custom changelog generator with its credits script and unit tests, the new Dependabot npm entry, and the four test tightenings from the previous round. I reproduced the release path locally (yarn changeset status, yarn changeset add --empty, yarn changeset version, which produced the 1.1.0 notes), the 177-test unit suite, and npm pack --dry-run, and checked the third-party claims the workflow comments rest on against primary sources.
- Changesets plus a release workflow — seven changesets under
.changeset/, a config wiring a custom generator, and.github/workflows/release.ymlpublishing onmainthroughchangesets/actionpinned to v2.1.2 with npm trusted publishing and noNPM_TOKEN.package.jsongainsworkspaces: ["."]so changesets sees the root package. - Custom changelog generator —
scripts/changelog.cjscites every commit that added or modified a changeset file (--diff-filter=AM) and thanks every non-maintainer, non-agent contributor;scripts/changeset-credits.tsreports the same attribution for whoever writes a credit line by hand;test/unit/changelog.test.tscovers attribution, agent and maintainer filtering, the shallow-clone fallback, and both release-line shapes. The end-to-end path works:changeset versionproduced correct notes from real history. - Dependabot npm entry — covers the root
yarn.lockand, through the Yarn workspace,test/e2e/package.json, with a 1-day cooldown, minor/patch grouping, and fivetypescript-*alias pins listed underignore. - Test tightenings from the last round — the template-end diagnostic test now asserts an exact anchor, the missing-plugin test is renamed and asserts its premise from the tsserver log, the scaling check hoists
createTemplateContextout of the timed region, and the message-reader test pins its multibyte split point. All four prior findings are resolved. - Doc and comment follow-ups —
CHANGELOG.md's hand-written Unreleased section is replaced by the changesets, themaskSubstitutionscomment no longer over-claims, andStyledPluginEmmetConfigurationis alphabetized.
ℹ️ Nitpicks
.github/dependabot.yml:15— "Rounds.yarnrc.yml'snpmMinimalAgeGate(minutes) up to whole days" describes a rounding that does not happen:1440minutes is exactly 1 day. The comment also reads as if the cooldown only tightens things, whiledefault-days: 1lowers Dependabot's built-in 3-day default for version updates.src/template/template-substitutions.ts:303— the rewrittenmaskSubstitutionscomment wraps mid-sentence ("Spans need / not arrive sorted or"), and oxfmt does not reflow comments for you.
Space Bunny (free) ｜ 𝕏
There was a problem hiding this comment.
ℹ️ No correctness blockers in the new commit — two informational notes below. This one cannot be an approval while the four threads from the last round are still open.
Reviewed changes
Reviewed the one commit pushed since the last review (a4104e2, 4 files): the scaling guardrail, the node10 self-link script, and the two doc paragraphs describing both. I ran yarn test:scaling end to end, forced its two new failure paths with a throwaway copy of the script, exercised every branch of the self-link script by hand, and ran yarn test:package-api:node10 on top of it.
- Every timed scaling result is now checked against its fixture —
defineChecktakes averifybesiderun, stops the clock before verifying, and reports a mismatch separately from the ratio, skipping the ratio entirely once a result is wrong, so a feature that throws (caught into an empty result, fast at every size) can no longer pass as a healthy operation. The expectations are derived from the fixtures rather than copied from a run: exact substituted text for all four placeholder shapes, thecolroffsets as diagnostic starts, one outlining span per rule, the hover span at offset 0 with the selector in the documentation, theredvalue entry, the rename fix's span, and one diagnostic per template in both many-template checks. - A plausibility floor on the ratio —
MIN_PLAUSIBLE_RATIO = 1.5fails a check whose 4N run is not meaningfully slower than its N run, with a message naming the two ways that happens. Measured here, all eleven checks land at 3.35-4.74: 2x above the floor, 1.5x belowSCALING_THRESHOLD = 7. Forcing the floor and a wrong expectation in a scratch copy produced the expected FAIL lines and exit code 1. - The node10 self-link is repaired instead of assumed —
ensure-node10-self-link.mjsrealpaths both sides, re-points a link that resolves elsewhere or dangles, and refuses to replace a real directory or file. All four cases in its header comment behave as written, including both failure exits. - Both doc paragraphs match the code —
docs/maintenance.md's scaling description anddocs/architecture.md's "Public surface" bullet now say what the gate and the script actually do.
ℹ️ The retained-heap guardrail reads bimodally, and the negative tolerance is nearly spent
RETAINED_HEAP_NEGATIVE_TOLERANCE_BYTES is half of RETAINED_HEAP_THRESHOLD_BYTES, so 7.00MB at the current budgets, and across four runs of yarn test:scaling the retained delta was +3.38MB, -6.55MB, +3.40MB, -6.55MB. It lands in one of two spots, and the negative one clears the tolerance by 0.45MB, so a different Node version or a differently loaded runner can push it past and fail yarn verify with a message blaming the probe. This did not come from this commit (the constant arrived in 3c53611, and stubbing out this commit's verify reproduces the same two readings), but it is the one remaining knife-edge in a file this commit just made trustworthy.
Technical details
# The retained-heap guardrail's negative tolerance has ~0.45MB of headroom on a bimodal reading
## Affected sites
- `test/performance/scaling-check.ts:517` — `RETAINED_HEAP_NEGATIVE_TOLERANCE_BYTES =
RETAINED_HEAP_THRESHOLD_BYTES / 2`, i.e. 7.00MB at `MAX_VALIDATION_CACHE_BYTES * 3 + 2MB`.
Introduced in `3c53611`, not in this commit.
- `test/performance/scaling-check.ts:577` and `:585` — the "before" and "after" readings, both taken
in the process that has just run eleven timed checks plus their verifications.
- `test/performance/scaling-check.ts:604` — the throw whose message concludes "the probe is broken".
## Evidence (Node 24.19.0, `a4104e2`, `yarn test:scaling`)
- Four clean runs: `+3.38MB`, `-6.55MB`, `+3.40MB`, `-6.55MB`, against a `14.00MB` threshold.
- Re-running a copy whose `verify` is stubbed to `mismatch: undefined`, so none of this commit's
added allocation runs: `-6.55MB`, `-6.56MB`, `+3.39MB`. The delta is not the cause.
- The guardrail is not measuring a smaller cache: every one of its `RETAINED_HEAP_EDIT_COUNT` (6000)
templates is distinct by its leading `/* edit N */` comment, so each is a cold
`RawValidationCache` miss, and the service is read again after the "after" gc specifically to
keep it reachable. The bimodality is heap state inherited from the timed checks.
## Required outcome
- The guardrail's readings stop depending on heap state inherited from the timed checks, or the
tolerance stops being a coin flip against a bimodal measurement.
## Suggested approach
- Run the guardrail in a child process, so its "before" reading is a fresh heap. A before/after delta
inside a process that has just done a megabyte-scale allocation storm is only meaningful when the
two readings are taken from the same starting state.
- Failing that, raise `RETAINED_HEAP_NEGATIVE_TOLERANCE_BYTES` to `RETAINED_HEAP_THRESHOLD_BYTES`
(still far below the positive threshold) and record in its comment that the reading is bimodal, so
a future reader does not read a 0.45MB margin as a measured bound.ℹ️ Nitpicks
- Two one-line descriptions of the gate still predate this commit:
AGENTS.md:78saysyarn test:scaling"gates superlinear time and retained memory in CI", and thedocs/maintenance.md:52commands table still reads "Guard against superlinear regressions in a hot operation". Since this commit it also fails a wrong result and an implausible ratio, and thedocs/maintenance.mdprose just below was updated for exactly that.
Space Bunny (free) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found. All four threads from the last round are genuinely addressed, and the new prose in this commit is the accurate version of the claims it replaced.
Reviewed changes
Reviewed the one commit pushed since the last review (208d029, 4 files, +32/−27): the release workflow, the Dependabot npm entry, the npmjs.com trusted-publisher instructions, and a new publishConfig.registry. Nothing runs in CI here, so I checked every load-bearing claim the new comments rest on against primary sources instead of taking the prose on faith: Yarn 4.18.1's publish.ts / npmConfigUtils.ts / npmHttpUtils.ts, @changesets/cli 3.0.3's yarn publish tool, dependabot-core's npm and Yarn file parsers, and GitHub's Dependabot options reference.
- Dependabot now protects the requirement it can actually see — the five
typescript-*entries are replaced by onedependency-name: typescript. The aliases really are invisible to Dependabot (dropped inignore_requirement?on the manifest path, skipped inYarnLock#dependencieswhiledealias_packages?is off on the lockfile path), while plaintypescriptis the name the live~6.0.3range resolves to inyarn.lock, so this entry is the one that bites. - The 1-day cooldown gives way to the platform default — dropping
cooldownis safe because Dependabot's documented default for version updates is 3 days (version updates only, so security updates are unaffected), which still clearsnpmMinimalAgeGate: 1440. - Two unused steps leave the release job — the
npm install -g npm@12pin and setup-node'sregistry-url.@changesets/cli3.0.3 selects its yarn tool, whosepublishrunsyarn npm publish --access … --tag …and whoseinforunsyarn npm info … --json, so the job never invokes thenpmbinary and the.npmrcthatregistry-urlwrites was never read. - Publishing now names its registry explicitly —
publishConfig.registryis honored:getPublishRegistryreturnsmanifest.publishConfig.registryahead of any scope or fetch setting, and the OIDC exchange posts to${registry}/-/npm/v1/oidc/token/exchange/package/…built from that same registry, which is exactly what the newdocs/maintenance.mdparagraph describes. No flag overrides it, since the yarn publish tool passes no--registry. - The Yarn OIDC floor is attributed to the versions that introduced it — 4.10.0 for the exchange, 4.10.3 for the scoped-package fix that this package needs, so
packageManager: yarn@4.18.1reads as a pin with slack rather than as the feature boundary. - The setup list tells the maintainer to permit direct publishing — step 3 names the
npm stage publishdefault for trusted publishers configured after 2026-09-03, so the first release no longer depends on an npmjs.com UI default this workflow does not match.
Also confirmed while reading around the delta: the PR description's compatibility section now matches the branch (CommonJS tsserver entry, TypeScript 5.0+, Node 14.21.3+, 177 unit and 131 e2e tests), which closes the one body-level item left over from the first review.
Space Bunny (free) ｜ 𝕏
There was a problem hiding this comment.
ℹ️ No correctness blockers in the new behavior — the substitution, wrapper, and completion work all hold up against the real CSS language service. Three inline notes are the spec and its comments describing that work inaccurately, plus one completeness question.
Reviewed changes
Reviewed the one commit pushed since the last review (765ab1d, "feat: adapt to styled-components v7 templates and modern nested at-rules", 16 files, +1029/−112): the substitution statement rule, the @layer prelude rewrite, the css value wrapper, nested at-rule completions, the spec sections describing all three, four changesets, and the new unit, e2e, and scaling coverage. I ran yarn verify end to end on this branch (format, lint, typecheck, 260 unit, 133 e2e, all five package-API type gates, npm pack --dry-run) plus yarn test:scaling (thirteen checks, ratios 3.51–5.07 against a threshold of 7, the two new ones at 3.71 and 4.49), then probed vscode-css-languageservice@6.3.10 and @vscode/emmet-helper@2.11.0 directly to confirm what each workaround is actually working around.
- A statement rule for block-position interpolations —
getTemplateSubstitutionsnow also reads a placeholder as a mixin when the last significant character before it, across lines, is;,{,}, or the template start, chaining that classification to a following placeholder, socolor: red; ${mixin},&:hover { ${mixin} }, and${a} ${b}stop reporting falsecolon expected/semi-colon expected. The excluded-next set keeps selector continuations, property names, and rule bodies x-filled; I ran 28 real-world template shapes through the real validator and saw no false diagnostics, with output length still equal to input length everywhere. - A nested block
@layerrewrite in the virtual document — a block@layerprelude in a nested position becomes a same-length&prelude, so the SCSS parser stops misparsing declarations in a layer body. The linked upstream issue is real, still open, and reproduces here; I also confirmed@layeris the only nested at-rule that misbehaves (declarations directly in@media,@supports,@container,@scope, and@starting-stylebodies already validate clean), so the newdocs/usage.mdclaim holds. - A value wrapper for value-shaped
cssfragments —:root{all:\nwhen the tag iscssand the substituted text reads as one value, memoized perTemplateContextin aWeakMapand folded into the validation cache key andcanReuseVirtualDocument, both of which stay sound because the substituted text is a pure function of the raw text. An unclosed value still reports at the template end, and any fragment with a;,{, or:keeps the declaration wrapper, so property completions survive mid-typing. - Nested at-rule completions — an
@or@meat a statement position adds nine at-rules carrying the CSS service's own documentation, cached once per service, skipping labels the CSS and SCSS lists already have, and replacing the whole at-keyword at the caret. I confirmed the catalog holds all nine names, that both language services really do return nothing usable at a nested@(CSS mode returns properties, SCSS mode only Sass directives), and that Emmet's@mediasnippet survives beside the new keyword entry rather than being duplicated by name. - Coverage for all of it — the new tests assert exact values derived from their own fixtures (substituted text, diagnostic start/offset/text, wrapper text and length, entry shapes and replacement spans) rather than counts or truthiness, and the two new scaling checks are sized so an injected quadratic would fail with margin, which the measured 3.71 and 4.49 confirm.
ℹ️ Nitpicks
- The PR description's "Verification" counts have drifted again: it says 177 unit and 131 e2e tests, and the branch now runs 260 and 133.
.changeset/nested-at-rule-completions.md:5groups all nine at-rules under "the at-rules that work there", but@keyframes,@property, and@font-faceare not valid nested in a style rule at all — they work only because v7 relocates them.docs/architecture.md:41already draws that split correctly, so the release note could mirror it.
Space Bunny (free) ｜ 𝕏
There was a problem hiding this comment.
Important
Two things to address before merging: the new shared CSS boundary scanner still diverges from the CSS language service in two of the places the new spec says it matches (one produces a false diagnostic, the other blanks a whole completion list), and the new createCodeLookback makes getTemplateSubstitutions quadratic for exactly the out-of-order spans its own contract accepts.
Reviewed changes
Reviewed the three commits pushed since the last review (090abd5, 1d90bc6, 5e3bc0a): the consolidated nonCodeEnd / createCodeLookback scanner and the two look-backs that consume it, the three added at-rules, scripts/check-pack-contents.ts plus its test:pack wiring, the retained-heap guardrail's child process, and the yarn pack and provenance changes in both workflows. I installed the branch and ran yarn test:pack (ok 14 files, exactly the expected set), yarn test:scaling (16 checks, ratios 3.31–4.69 against a threshold of 7, retained heap +3.87MB) and yarn test:unit (295 passing, matching the description), packed into a directory that did not exist, and drove the plugin's own diagnostics and completion paths against the pinned vscode-css-languageservice@6.3.10. Two specialists verified the YARN_NPM_PUBLISH_PROVENANCE chain through Yarn 4.18.1 and @changesets/cli 3.0.3, and hunted for scanner divergences.
- One boundary scanner for four call sites —
nonCodeEndwithcommentEnd/stringEnd/escapeEnd/lineBreakEndreplaces four ad-hoc scanners, andcreateCodeLookbackserves the statement rule and the at-keyword look-back.isLineBreaknow includes\fandescapeEndimplements the CSS hex-escape rule; both are real fixes, and I confirmed 6.3.10 agrees (scssScanner.js:92-98,cssScanner.js:443-483). The look-back'srunStart/runEndmemo survives a shuffled query order (637k queries, zero mismatches), and every sorted-input shape measures linear. Two gaps survive, in the inline below. - The statement rule looks back past comments — the mixin branch asks the look-back instead of a regex over the text before the placeholder, so
color: red; /* note */ ${mixin};now becomes$a:0;, and a placeholder inside a comment or string stays x-filled whatever that string contains.followsDeclarationBoundaryfolds in the previous-placeholder case, which is what keeps${a} ${b}two mixins. - At-keyword completions skip comments, and the hoisted set is complete — the list is now suppressed inside a comment or string and offered after one (
color: red; /* note */ @), and@page,@counter-style, and@font-palette-valuesjoin the set. The spec's inclusion rule does justify all three, so last round's completeness question is answered on the merits. - An exact pack-contents gate —
scripts/check-pack-contents.tsruns the same packeryarn npm publishuses, derives the expected set frompackage.jsonplus the two build-hook files, and fails listing every missing and unexpected path; it replacesnpm pack --dry-runinverifyand CI ahead of a realyarn pack. I confirmedyarn pack --outcreates missing parent directories, so dropping CI'smkdir -p package-artifactis fine, and thatyarn packstill runsprepack, so the gate never reads a stalelib/. - The retained-heap guardrail gets a fresh process — the child-process split with a distinct exit code for "over budget" closes the bimodal reading I flagged two rounds ago, and the tolerance comment now explains the negative mode that survives even in the child rather than pretending it does not.
- CI and release move to Yarn's packer —
npm packis gone from both workflows, provenance is on, and the two stale one-line gate descriptions inAGENTS.mdanddocs/maintenance.mdnow say what the gate actually fails on.
Space Bunny (free) ｜ 𝕏
There was a problem hiding this comment.
Important
Three places where the new escape reading, the new caret classifier, and the new interpolated-name fill leave a false diagnostic or a bad completion in a shape one token away from the ones their own tests pin. None is a regression against 1.0.1, but all three sit in code these commits wrote, and two contradict claims the new changesets make.
Reviewed changes
Reviewed the three commits pushed since the last review (1b090c2, 8be45b4, 50de41e): the completed CSS-tokenizer escape handling, the new src/virtual-document/javascript-escapes.ts and its wiring into every structural decision, the new substitution branches for joined property names and at-rule conditions, the findCaretPlacement classifier, the configuration normalizer that reports rejected values, the scaling retry plus five new timed checks, and the compile:unless-fresh build-once plumbing. I installed the branch and ran yarn test:unit (529 passing), yarn test:e2e (143 passing), yarn test:scaling (21 checks, ratios 3.15–4.60 against a threshold of 7, retained heap +3.86MB), yarn test:pack (ok 14 files), format:check, lint, and typecheck, then drove the real plugin pipeline against the pinned vscode-css-languageservice@6.3.10 to reproduce each inline finding. Two specialists independently audited the escape stand-ins and the caret classifier; their two substantive findings are verified inline below.
- Escapes at code positions are now followed, and the spec says so —
nonCodeEndreturns the whole identifier run a backslash starts or continues (startsCodeEscape,identifierEnd), the look-back counts an escape as one significant character at its backslash, andopensUrlArgumentdecodes escapes to recogniseurlandurl-prefixthe way the parser does. This is the backslash half of the last round's scanner finding; the/*-inside-url()half of that thread is unchanged, and the spec bullet now describes it more precisely rather than differently. The new unterminated-run rule (isUnterminatedRun) is what lets a caret at the end of an unclosed string or comment keep typing into it, and the new unit block exercises that end-of-run case rather than only in-string escapes. - Templates are validated as the CSS styled-components receives —
replaceJavaScriptEscapesturns each run of adjacent escape sequences into a same-length stand-in for its cooked text, and the masked text in substitution, the virtual document, the value-shape test, and the stray-brace search all read that. The arithmetic closes: 210k fuzzed inputs preserved length exactly, and the cooked values agree with a realevalcook across every escape shape I checked, invalid ones included. Spending the padding inside a CSS escape where CSS allows it ("\\f101"→"\0f101",u\\rl(→u\72l() is a nice touch; the one place it is not token-safe is inline below. - Interpolated names, at-rule conditions, and values split over lines — a placeholder joined to name characters becomes the Sass interpolation
#{x}so the linter never reports an unknown property, one in an@media/@supports/@containerprelude after a combinator becomes(x…)orx(…), and a value written one interpolation per line keeps the second a value instead of a dummy declaration. The one-parameter look-back (one gap, one word) and the per-query-kind run scanners keep all of this linear: the three new substitution checks measure 4.20–4.34, and the new diagnostics-with-escapes check measures 4.41. - Completions and Emmet land where they fit —
findCaretPlacementreturnsNonCodeinside a comment or string,NoDeclarationin a value, a pseudo-class selector, a media feature, or an at-rule prelude, andOtherelsewhere; an empty template now offers property suggestions. The spec's cost claim holds: I could not build a shape that needs more than the two documented passes over the text before the caret. - Rejected configuration is reported instead of ignored — per-field normalization logs one line naming the setting, what it accepts, and what it received, accepts a handful of unambiguous aliases, and suggests the nearest setting for a misspelled key by edit distance. A logger that throws cannot stop the configuration from applying, and the
name/globalkeys tsserver adds log nothing. - Contributor plumbing — a scaling ratio outside
[1.5, 7]is re-measured up to twice so machine load alone cannot failverify, while a wrong result still fails at once;compile:unless-freshplusSTYLED_PLUGIN_LIB_FRESHmeansverifyand CI's compile job buildlib/once and every later script reuses it, with thelib/index.js-exists guard keeping the package-API runtime test honest after it deleteslib/;.gitattributesforces LF, which is what the exact-offset e2e fixtures need;changelog.cjspipes git's stderr so a failure message reaches the caller instead of the terminal, andchangeset-creditsnow throws with git's error rather than reporting "no history".
ℹ️ The description's test counts drifted again
"Verification" still says 295 unit tests and 133 real-tsserver end-to-end tests. On this head the suites are 529 and 143, so the number a reader uses to gauge the change is off by more than half.
Note: 4 inline comment(s) dropped because they did not anchor to lines inside the PR diff:
src/features/completions.ts:6876(RIGHT) — line 6876 (RIGHT) is not inside a diff hunksrc/virtual-document/javascript-escapes.ts:9213(RIGHT) — line 9213 (RIGHT) is not inside a diff hunksrc/template/template-substitutions.ts:8335(RIGHT) — line 8335 (RIGHT) is not inside a diff hunkdocs/architecture.md:2291(RIGHT) — line 2291 (RIGHT) is not inside a diff hunk
Space Bunny (free) ｜ 𝕏
|
The review above covers
|
There was a problem hiding this comment.
Important
Two things to address before merging: the new single-identifier validation filter breaks the soundness of the per-file validation cache key, and a property name followed by a rule on the same line is still read as a selector.
Reviewed changes
Reviewed the two commits pushed since the last review (f23fc0e, 4c5e8aa, 25 files): the escape-run bookkeeping that widens diagnostics, hover, folding, and completion spans, the name-aware and url-aware padding in replaceJavaScriptEscapes, the single-identifier css reading with its intersection filter, the nextSpecialInList selector-list continuation, the keyframe % and @media … or placeholder shapes, normalizeSpans for ./api callers, the compare-release sensor, and the spec and test changes describing all of it. I ran the branch green end to end (format:check, lint, typecheck, 649 unit, 147 e2e, test:scaling at 28 checks with ratios 3.00-4.71 against a threshold of 7 and retained heap 3.88MB, test:pack at 14 files, and compare-release itself at 32 cases with 20 differing from 1.0.1), and drove the real plugin pipeline against the pinned vscode-css-languageservice@6.3.10 to reproduce each finding below.
- Escape runs, and spans widened to whole runs —
getTemplateEscapeRunsrecords the raw span each stand-in covers in the same pass as the replaced text and rides onTemplateLineMapasescapeRuns;fromVirtualDocRange(diagnostics, hover, folding),fromVirtualDocRangeStrict(code-fix edits), andwidenToEscapeRuns(completion replacement spans) then resolve each end and widen it, and a fix whose edit overlaps a run is dropped.findFirstRunEndingAftermakes each lookup a binary search, and collapsing fourfromVirtualDocPositioncall sites into onefromVirtualDocRangeper feature is a real readability win. I fuzzed 400k generated escape, name, and url inputs: no throw, length always equal to the input, runs always ascending and disjoint. - Padding that no longer splits a name or a url token — the new
openNamestate machine moves a run's padding in front of the whole name (co\x6Crreads ascolr,.md\\:flexas.md\:flex,@me\x64iaas@media), falls back to a CSS hex escape where whitespace would split a larger token (&.bt\x6Eas&.bt\06e,a:ho\x76erasa:ho\76 er), and writes_inside an unquotedurl()where a space would end the token.createUnquotedUrlLookbackreuses the previous run's answer so each stretch of text is read once. This closes the prior round's escape-inside-url()finding:url(a\\(b\\).png)no longer reports) expected, and the caret no longer mistakes a;inside a data URI for a statement boundary. - A single-identifier
cssfragment reads as a value too —getCssFragmentShapereturns a third shape, the provider keeps the declaration wrapper for completions and handscreateValueReadingDocumenta second document under:root{all:, and diagnostics keep only what both readings report, socss`none`,css`${x}px`, andcss`colr`are silent whilecss`colr: red;`still reports. The fake-language-service test proving a diagnostic both readings share survives, and the cache-hit count test still holds. - The url() boundary scanner now matches the parser's trivia, token, trivia reading —
urlArgumentRunEndopens a block comment only where a token could start, keeps//off, steps over an escape, and makes every character inside the argument url content forfindStrayClosingBraceOffset,getCssFragmentShape, and the look-back, which answers the argument's(. This closes the remaining half of thedocs/architecture.md:37thread, and the three prior findings that were still open on50de41eare now genuinely fixed. - More placeholder shapes —
${step}%fills with zeros, a selector list split over lines ending in,is read as selectors,@media ${a} or ${b}and preludes behind comments are conditions, andnormalizeSpanssorts, clamps, and merges an./apicaller's spans so the output depends only on positions. Seven new scaling checks cover the list scanner, the unterminated-comment skipper, reversed spans, and the two escape paths; all measure linear (3.47-4.29). compare-release, a sensor against a published release —scripts/compare-release.tsand its default cases run each template throughlib/and 1.0.1 the way tsserver loads a plugin and print what differs. It works, and its output is the most legible account of this PR's behavior change anywhere in the repo.
ℹ️ The escape-replacement invariants have no property test, and this delta added the branching that could break them
replaceEscapes grew three new interacting pieces of state (openName, hexEscapeLastCharacter, createUnquotedUrlLookback), and every table case asserts result.length === raw.length individually, so nothing checks the invariant the whole plugin's offset mapping rests on. I fuzzed 400k generated inputs and it holds today, which is why this is informational rather than a blocker, but the check is cheap and the failure mode is silent: a length that drifts by one shifts every diagnostic, hover, fold, and edit in that template.
Technical details
# The escape-replacement length and run-span invariants are only asserted per case
## Affected sites
- `src/virtual-document/javascript-escapes.ts:92` (`replaceEscapes`) — the new `openName`
lifecycle, the `hexEscapeLastCharacter` fallback, and the `URL_PADDING` branch; the
stand-in a run produces and the `[start, end)` pushed to `escapeRuns` come out of three
different paths that must agree.
- `test/unit/javascript-escapes.test.ts:16` — the tables assert length per row; nothing
iterates.
## Required outcome
- One test that generates inputs from the pieces the new branches branch on (a name
character, a `NAME_LEADER`, a `\` run, `url(` or `url-prefix(`, a string, a comment, a
digit, an astral character, a line break) and asserts, for each, that the output length
equals the input length and that the recorded runs are ascending, disjoint, and inside the
input.
## Open questions for the human
- Whether that belongs in `test/unit` or in the scaling check's `verify` hook, which already
asserts exact expected text per size.
ℹ️ Nitpicks
docs/architecture.md:107— "another value is rejected and the key dropped, so the CSS language service's own default for that rule applies" is false for the one key the plugin defaults:updateFromPluginConfigspreads...defaults.lintbefore the normalizedlint(src/configuration/plugin-configuration.ts:116),defaultConfiguration.lintis{ emptyRules: 'ignore' }, andvscode-css-languageservice's own default foremptyRulesisWarning(node_modules/vscode-css-languageservice/lib/umd/services/lintRules.js:44). A mistypedemptyRulestherefore lands onignore, notwarning.
Space Bunny (free) ｜ 𝕏
There was a problem hiding this comment.
Important
The ; / } stop that fixes the one-line property-name case also fires inside comments and strings, so a selector interpolation with a comment holding a ; is now read as a property name and reports false diagnostics. 33 such templates where 1.0.1 is silent, inline below.
Reviewed changes
Reviewed the one commit pushed since the last review (ebe3558, 16 files): the ; / } stop added to the placeholder position classifier, the rewritten unquoted-url() branch of replaceJavaScriptEscapes with its carried insideUrl state, case-insensitive lint-level acceptance with a log message that names the default in force, and the docs, changesets, and tests for all of it. The branch is green locally (format:check, lint, typecheck, 676 unit, 149 e2e), and I drove the plugin's own compare-release sensor against 1.0.1 to judge each change in the direction that matters.
- A statement boundary ends the selector search —
isSelectorScanStopis now shared bynextSpecialandcreateListContinuationScanner, so a;or}between a placeholder's colon and the rule's{/&ends the declaration.padding-${s}: 0; &:hover { … },${p}: 0; &:hover { … }, and the}-then-&shape all keep the property-name fill, each with unit, e2e, andcompare-releasecoverage; I confirmed 1.0.1 reportedcolon expected/semi-colon expected/} expectedon every one of them. The one;-then-{and;-then-&rows pin the new behavior rather than a tautology, and the new}stop is what makes&:focus { ${p}: 0 } &:hover { … }come out right. - A url token stays whole across an escape — the url branch no longer requires a trailing escaping backslash, so it now outranks the name branches, and the padding moved after the cooked text:
url(a/\x41y.png)reads asurl(a/A___y.png),url(x\u0041y.png)asurl(xA____.png), and a line continuation asurl(foo__bar.png). The look-back takes the previous run's answer across its cooked text unless that text holds whitespace, a quote, or a paren, which keeps the read linear. A specialist fuzzed 400k generated inputs: output length always equals input length, run spans always ascend and stay disjoint, and the new stand-in reproduces the cooked text's diagnostics in all 14 shapes it could construct where4c5e8aadid not. The line continuation inside a url, which made 1.0.1 throwDebug Failure. False expression., is now clean on both LF and CRLF, and the new unit test asserts the followingcolrdiagnostic's exact offset across both. - Lint levels in any letter case, and a log line that names the default —
toLintLevellowercases before thehasOwnlookup, so"Error","OFF", and"Warn"are accepted silently, and a rejected value now says which default actually applies (The plugin default, "ignore", applies.for a key indefaultConfiguration.lint,The CSS language service's own default for this rule applies.otherwise).normalizeLinttakesdefaultsas a parameter instead of re-deriving the answer, which also retires thedocs/architecture.md:107nitpick from the last round: the spec,docs/usage.md, and both changesets now say the accurate thing.
ℹ️ Nitpicks
src/template/template-substitutions.ts:118— the rewordedSyntaxBoundaryScannerdoc now runs the clause "where the next{,&,;,}, or line terminator after it is (isSelectorScanStop). Each query scans lazily…" past the comment's right margin, so oxfmt leaves a mid-sentence wrap. Reflowing that one line is enough.
Space Bunny (free) ｜ 𝕏
There was a problem hiding this comment.
ℹ️ No correctness blockers in this delta — three suggestions inline, in a release note, a comment trim, and a build script.
Reviewed changes
Reviewed the three commits pushed since the last pullfrog review (77bd1fa, 99bbdf9, 218f0d4; 16 files, +575/−471): the package-API gate scripts consolidated into two TypeScript entry points, the line-map module split out of the virtual-document provider, the top-level nesting mode added to the reuse rule and the validation cache key, and the hover span clamp. I ran the unit suite (681 passing) and the real-tsserver e2e suite (149 passing) on the branch, and proved the new tests can fail by stubbing isTemplateTopLevelNested to a constant: the provider test's two isFalse assertions and both @layer diagnostic orders go red.
- Cache modes are isolated end to end —
isTemplateTopLevelNestedis a new public method onStyledVirtualDocumentProvider,canReuseVirtualDocumentnow requires the same mode, andbuildValidationCacheKeywrites it as one character after the length-prefixed wrapper, so a same-text component template and global-style template no longer share a validation entry, a parsed document, a line map, or a completion result. Every other consumer of the reuse rule (the completions cache,CachedVirtualDocumentSessionProvider, and the newgetReusableLineMapfast path) inherits the fix throughcanReuseVirtualDocumentrather than re-deriving it, and the key's length prefix still keeps(wrapper, mode, rawText)tuples from concatenating together. - The line map moves into its own module —
src/virtual-document/template-line-map.tstakes the position-only helpers,positionsEqual, and the line-terminator constants; the provider keeps virtual-document construction and CSS scanning and re-exports the seven names its consumers import. The move also drops the provider's privatepositionsEqual, leavingcompletions.tsas the only second copy.VirtualPositionMapperkeeps the split one-directional, so the new module depends on nothing that imports it. - A hover with no range no longer overruns the template — the fallback length is
Math.min(1, context.text.length - start), which is 0 at the template end and 1 everywhere else; the anchored position is already clamped to the template end byfromVirtualDocPosition, so the length can never go negative. - Package-API gates consolidated — the inline
node -efreshness check pluscheck-typescript-alias.mjsandensure-node10-self-link.mjsbecomescripts/compile-unless-fresh.tsandscripts/test-package-api-types.ts, with oneCHECKSentry per gate. I compared each deleted script against the new path: the alias check, the node10 self-link (including all four of its link states and both failure exits), the per-versiontscinvocation, and the compile-first ordering are all still performed, with the same failure messages, and spawningprocess.execPathinstead of shelling out toyarn compileis what makes the gate work on Windows.
One thread from the last round is half addressed; I replied on it with what I measured at 218f0d4 and left it open, since the value-reading collision is unchanged.
Space Bunny (free) ｜ 𝕏
There was a problem hiding this comment.
ℹ️ No correctness blockers in the refactor — two small coverage gaps inline.
Reviewed changes
Reviewed the one commit pushed since the last review (0abc58c, 88 files, +6661/−7501): a whole-branch "simplify" pass that extracts the shared CSS boundary scanner into src/virtual-document/css-code-scanner.ts, folds the mapping predicates into one span-based pair, routes code fixes through the diagnostics list, moves getTemplateSettings out of the tsserver entry, and rebuilds the CI, packaging, and test harnesses around one composite setup action.
- The validation cache key now determines the stored result —
getReadingKeyreturns${isSingleIdentifier}${topLevelIsNested}${wrapper}, so acsssingle-identifier fragment and a component template with byte-identical text no longer share an entry. Re-measured with oneStyledTemplateLanguageServiceon the textred: isolatedcss→ 0 andstyled.div→ 2, and cached in both orders the pairs come back[0, 2]and[2, 0]. This closes the thread from the last round. - One shared boundary scanner for every structural reader —
nonCodeEnd,createCodeLookback,createScanCandidatePattern, and the character predicates move intocss-code-scanner.ts, and the four modules that had each grown their own copy now import from there. Two specialists fuzzed the result against218f0d4:replaceJavaScriptEscapesis byte-identical over roughly 8M inputs apart from one documented change, andgetTemplateSubstitutionsis identical over 920k pairs apart from the four documented fill changes, with output length equal to input length and run spans ascending and disjoint throughout. It also removes two pre-existing./apibugs, a length violation and aRangeErrorfrom an empty span. - Name characters now match
vscode-css-languageservice's_identChar— every code unit ≥ U+0080 except U+2028/U+2029, so a non-ASCII space joins a name where it used to split one. I checked the spec's claim againstnode_modules/vscode-css-languageservice/lib/esm/parser/cssScanner.js:571; the spec states the exception and the reason for it. - Mapping is span-based end to end —
fromVirtualDocSpan/fromVirtualDocSpanStrictreplace the range-returning pair, and folding, hover, diagnostics, and code fixes all take template offsets from them instead of round-tripping throughTemplateContext.toOffset, which retires the last per-resulttoOffsetcalls on the interactive paths. - Code fixes read the shown diagnostics —
CodeActionsFeaturenow takesDiagnosticsFeature, so a request is answered from the validation cache, covers only diagnostics the editor can see, and honorsvalidate: false. - Tooling, CI, and harnesses rebuilt —
prepackisyarn compileandscripts/compile-unless-fresh.tsis gone; the seventest:package-api:*scripts collapse intotest:package-api:consumers; one.github/actions/setupaction replaces five copies of the setup-node/Corepack/Yarn preamble; the retained-heap guardrail runs in process instead of in an untimedspawnSyncchild; and the e2e suite shares one tsserver per suite file, which takes a full run from about two minutes to thirteen seconds. The rewritten scenarios kept exact-value assertions and a positive control in every "returns nothing" test, so the 2266 deleted lines of test code bought 10 more tests rather than looser ones.
Space Bunny (free) ｜ 𝕏
|
Run failed. View the logs →
|
…ed CI and release pipeline Package builds as a CommonJS tsserver entry at 1.0.1's path plus a dual CommonJS and ESM ./api. CI builds and packs once, then runs the e2e matrix, the Node floor smoke test, and the scaling guardrail; releases run the full verify before Changesets can publish. Every test and measurement tool runs under a wall-clock deadline and a heap cap. Co-authored-by: usercao <cao858646882@qq.com>
…working styled-components code Reorganizes the plugin into configuration, template substitution, virtual document, tsserver, and one-file-per-feature modules while keeping 1.0.1's public constructor, mapping contracts, and TypeScript 5.0 floor. Interpolations in property names, at-rule conditions, selectors, and keyframe percentages are read by their role, comments and strings no longer mislead that reading, and JavaScript escapes are checked as styled-components receives them. Adapts to styled-components v7 templates, maps positions exactly, caches validation within a size bound, and keeps the common path free of edge-case work. Co-authored-by: usercao <cao858646882@qq.com>
…d scaling behavior A shared tsserver per suite file runs every scenario across the supported TypeScript range; package tests consume the packed tarball from CommonJS and ESM and type-check consumers on TypeScript 5.0 through the current release; the scaling check guards superlinear time and retained memory. Co-authored-by: usercao <cao858646882@qq.com>
…add AGENTS.md Co-authored-by: usercao <cao858646882@qq.com>
b33fb9f to
d8fc730
Compare
|
Alright, I think this is ready to roll. Waiting for the final review. |
A delay above 2147483647 ms makes Node fire the timer after 1 ms, so an oversized TSSERVER_RESPONSE_TIMEOUT_MS or WITH_DEADLINE_MS failed the run at once instead of extending it.
|
Thank you so much @usercao :) a prerelease is available for you test: https://github.com/styled-components/typescript-styled-plugin/releases/tag/%40styled%2Ftypescript-styled-plugin%401.1.0-prerelease-20260927193731 |
|
Run failed. View the logs →
|
|
@quantizor Thank you very much for all your hard work on this! With this preview release available, I can now start working on the bugs in Thanks again for your time and effort! |
Hi @quantizor, I have prepared this update to bring the project up to date while preserving its existing purpose and public API.
This PR keeps the project's role as a cross-editor TypeScript Server plugin. The work is intended to make the plugin easier to maintain, test, and publish, while also addressing several compatibility and language-service issues found during the update.
What changed
lib/index.jspath, so every supported host loads it with a synchronousrequire()and norequire(esm)interop;./apiships a real CommonJS build forrequire()and an ESM build forimport().globalCssandkeyframestemplates, completion edit ranges, cache invalidation, and disabled validation. Completion entries keep 1.0.1's shape: the editor inserts the entry's label.StyledPluginConfigurationInputtype covers partial configuration, and the lint API now also exposesunknownAtRules.request_seq, and failing tests when tsserver exits unsuccessfully or leaves requests incomplete.injectGlobal, and disabled Emmet behavior.prepackbuild so a clean Git checkout cannot publish an empty shell package. Package tests remove existing build output, create and unpack the tarball, synchronously load the tsserver entry, import and require the API, and exercise the legacy code-fix API from the packed artifact.Compatibility notes
This is a minor release with no breaking changes for 1.0.1 users. The plugin supports TypeScript 5.0 and newer (6.x recommended) and Node.js 14.21.3 or later in the tsserver host. TypeScript 5.0 is the range 1.0.1 actually ran on: on older hosts 1.0.1 crashes during activation, while this version logs a clear message and leaves the host's language service untouched. The Node floor is the one 1.0.1's own runtime dependencies already require.
The tsserver entry, its types, the
lib/apideep imports, andStyledPluginConfigurationkeep 1.0.1's exact shapes, and a type-check gate compiles a consumer written against 1.0.1 on every supported TypeScript version. The public language-service API is also available through the new./apiexport. Legacy three-argument code-fix calls remain supported, and custom virtual-document providers can opt into cache reuse. VS Code with workspace TypeScript is the primary verified host; other tsserver-compatible editors should be listed as supported only after host-specific validation.Verification
The repository provides a single
yarn verifyworkflow covering formatting, linting, strict type checking, 873 unit tests, 159 real-tsserver end-to-end tests, a scaling and retained-memory guardrail, package API type checks on TypeScript 5.0 through the current release (including a 1.0.1-shaped consumer andnode10andbundlerresolution), packed-artifact runtime consumption, and an exact check of the filesyarn packputs in the tarball. CI builds and packs once on the Node.js version in.github/.node-versionand runs the end-to-end suite on Node.js 22.12 with TypeScript 5.0.4, 5.9, and the current release, and on Node.js 24 with the current release. Separate jobs drive a real tsserver from the packed tarball on Node.js 14.21.3 with TypeScript 5.0.4, and prove graceful non-activation on TypeScript 4.9.5. Performance benchmarks are also included to track completion latency and retained heap behavior. Every test and measurement tool runs under a wall-clock deadline and a heap cap, so a regression fails loudly instead of hanging the machine.Next steps
After this PR, I plan to address the currently open issues in this repository. Once those have been handled, I plan to update and maintain the downstream styled-components/vscode-styled-components repository.
Please merge this PR. If you have a different direction in mind or would like any part of the update adjusted, please let me know and I will update it accordingly.
Thank you for your time and review.
Maintainer follow-up
Thank you so much @usercao for this. I built on your work and compacted the branch into four commits (build, language service, tests, docs), each co-authored by you, so the release can ship as a minor with no breaking changes for 1.0.1 users:
lib/apiimports.;,{,}, or a comment) are read as mixins, value-shapedcssfragments validate as values, a nested@layerwith declarations no longer reports{ expected(a workaround for SCSS: nested @layer with declarations reports '{ expected' microsoft/vscode-css-languageservice#530), and@at the start of a statement suggests the at-rules that work in a component.margin-${side}), at-rule conditions (@media screen and ${q}), keyframe percentages, selector lists split over lines, multi-line values, a;or{inside a nearby comment or string, and JavaScript escapes, which are now read the way styled-components receives them. Emmet no longer suggests declarations inside values, strings, or comments, and a mistyped or misspelled plugin setting now logs what was ignored and the accepted form.yarn compare-releasecompares the working build against a published release.AGENTS.mdand an architecture spec, and set up Changesets with a release workflow that runs the fullyarn verifybefore it can publish; release notes now live in.changeset/.