Skip to content

Modernize the plugin and strengthen language-service compatibility - #33

Open
usercao wants to merge 56 commits into
styled-components:mainfrom
usercao:chore/modernize-project
Open

usercao wants to merge 56 commits into
styled-components:mainfrom
usercao:chore/modernize-project

Conversation

@usercao

@usercao usercao commented Sep 16, 2026

Copy link
Copy Markdown

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

  • Migrated the development workflow to Yarn 4 and replaced the previous build, lint, formatting, and test tooling with tsdown, Oxlint, Oxfmt, and Vitest.
  • Updated the package for TypeScript 6 and current language-service dependencies, with an ESM-only package that retains the synchronous factory contract required by tsserver through Node.js require(ESM) interoperability.
  • Reorganized the implementation into focused modules for configuration, template substitution, virtual documents, tsserver integration, and individual CSS language-service features, while preserving the public constructor and mapping contracts.
  • Expanded unit and tsserver end-to-end coverage for completions, completion details, diagnostics, code fixes, hover, folding, Emmet, configuration updates, plugin lifecycle, script kinds, styled-components syntax, interpolation mapping, and the published package API.
  • Improved handling of dynamic CSS declaration names and values, globalStyle and keyframes templates, completion edit ranges, cache invalidation, and disabled validation. Completion translation now preserves insertion text, snippet semantics, filtering text, and optional replacement spans from the CSS language service.
  • Fixed CSS code actions at diagnostic start positions, retained the original three-argument public code-fix call, and ensured explicitly filtered requests only return fixes for the plugin's diagnostic code.
  • Hardened public extension boundaries by validating virtual-document mappings through the provider contract instead of assuming a fixed wrapper or trailer, safely accepting unknown object-shaped or partial plugin configuration from tsserver, and exporting separate input and resolved configuration types. The lint API now also exposes unknownAtRules.
  • Preserved all ECMAScript line terminators during template substitution. U+2028 and U+2029 are normalized only in the virtual SCSS document, keeping UTF-16 offsets stable while aligning TypeScript and CSS language-service line maps.
  • Reduced repeated work on interactive completion and hover paths by reusing virtual documents and parsed stylesheets. The provider now controls cache reuse explicitly, with built-in invalidation across text, wrapper, and file changes and conservative behavior for third-party providers.
  • Hardened the real-tsserver harness by resolving fixture paths with native ESM APIs, parsing protocol frames as raw UTF-8 bytes, escaping Unicode line separators in the line protocol, matching responses by request_seq, and failing tests when tsserver exits unsuccessfully or leaves requests incomplete.
  • Added focused regression coverage for completion-source merging, duplicate candidates, SCSS filtering, unknown completion details, hover and folding fallbacks, diagnostic severity and code translation, invalid code actions, cross-file cache isolation, injectGlobal, and disabled Emmet behavior.
  • Added a prepack build 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 the ESM API, and exercise the legacy code-fix API from the packed artifact.
  • Refreshed the README and split usage and maintenance guidance into dedicated documents, including the supported host contract and release workflow.
  • Updated CI to use Node.js 24.21 for development and builds, then run the compiled artifact against the Node.js 22.12 runtime minimum and the minimum and current supported TypeScript 6 releases. CI also passes the actual npm tarball from the build job to Node.js 22.12 and verifies both the synchronous plugin entry and ESM ./api export there. The same formatting, linting, type-checking, test, package API, and package-content checks remain available locally.
  • Added a 24-hour Yarn package-age gate so newly published dependency versions are not selected immediately during dependency resolution.

Compatibility notes

Because this update moves the package to TypeScript 6 and an ESM-only distribution, it is intended for a new major release. The package requires TypeScript 6.x and Node.js 22.12 or later so that tsserver hosts can synchronously load the ESM entry point. Development and builds use Node.js 24.21, while the compiled plugin continues to be tested on the Node.js 22.12 runtime minimum. VS Code with workspace TypeScript is the primary verified host; other tsserver-compatible editors should be listed as supported only after host-specific validation.

The public language-service API remains available through the ./api export. Legacy three-argument code-fix calls remain supported, custom virtual-document providers can opt into cache reuse, and ResolvedStyledPluginConfiguration is available for consumers that need the manager's fully populated configuration. The tsserver root entry now provides condition-specific ESM and CommonJS declarations, and package-level tests verify ESM API consumption, CommonJS TypeScript consumption, and synchronous runtime loading from the packed artifact.

Verification

The repository now provides a single yarn verify workflow covering formatting, linting, strict type checking, 91 unit tests, 105 real-tsserver end-to-end tests, clean-output package API checks, packed-artifact runtime consumption, and npm pack --dry-run. CI builds once on Node.js 24.21 and verifies the resulting artifact on Node.js 22.12 with TypeScript 6.0.2, Node.js 22.12 with the current TypeScript 6 release, and Node.js 24.21 with the current TypeScript 6 release. Performance benchmarks are also included to track completion latency and retained heap behavior.

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.

Copilot AI 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.

🟡 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 globalStyle tag is only checked as configuration data. The real-tsserver fixture overrides tags without globalStyle, and the styled-components syntax matrix does not exercise it, so the advertised host behavior can regress unnoticed. Add globalStyle to 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

Comment thread src/template/template-substitutions.ts Outdated

Copilot AI 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.

🔵 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
  • translateCompletionEntry intentionally omits replacementSpan when an LSP completion has no textEdit (see test/unit/template-language-service.test.ts:108-117), but this protocol model requires the field for every completion. Any E2E assertion that reads item.replacementSpan for 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

Copilot AI 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.

🔵 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_seq only 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 by request_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 call shutdown(). If tsserver drops a response or remains alive without answering, the close event 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

Copilot AI 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.

🔵 Needs a closer look

src/features/diagnostics.ts must honor validate: false before approval.

Review details

Suppressed comments (1)

src/features/diagnostics.ts:25

  • validate is normalized and updated by the configuration manager, but this feature never consults it before calling doValidation. As a result, validate: false still produces CSS diagnostics (the new plugin-lifecycle scenario at test/e2e/scenarios/plugin-lifecycle.test.ts:69 will 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

Copilot AI 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.

Note

Copilot was unable to run its full agentic suite in this review.

Pull request overview

Copilot reviewed 100 out of 108 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/index.ts:1

  • This export is not valid TS/ESM syntax (export { x as 'string' } is not allowed), and it also risks breaking the tsserver plugin loader contract + mismatching types/index.d.cts (which declares export = createPlugin). Use a valid export shape and keep runtime + typings aligned (e.g., export default createPlugin + update index.d.cts accordingly, or provide a dedicated CJS shim entrypoint in exports.require that sets module.exports = createPlugin).
    src/template-language-service.ts:1
  • Constructor overload signatures must be terminated (typically with ;) and cannot appear as bare constructor(...) declarations without a body. As written, this will not parse/typecheck. Add proper overload signature terminators (or remove overloads and use a single signature with discriminated arguments).
    test/package-api/runtime.ts:1
  • The code calls pluginFactory(...) before verifying that pluginFactory is actually a function. If the export shape changes (which is likely given the ESM/CJS interop changes in this PR), this will throw a generic runtime TypeError before the intended diagnostic. Check typeof pluginFactory === 'function' first, then invoke it.

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