refactor: align Lynx terminology with official specification#2276
refactor: align Lynx terminology with official specification#2276
Conversation
🦋 Changeset detectedLatest commit: 9e46237 The changes in this PR will be included in the next version bump. This PR includes changesets to release 15 packages
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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR renames template-centered public types and interfaces to bundle-centered names across multiple packages (e.g., LynxTemplate → LynxBundle, TemplateHooks → BundleHooks, LynxTemplatePlugin → LynxBundlePlugin), adds deprecated aliases, and updates documentation to use Main Thread Script (MTS) / Background Thread Script (BTS) terminology. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR adds official API aliases (e.g., LynxBundle, BundleLoader, LynxBundlePlugin, BundleHooks) for legacy internal names to align with the official lynxjs.org specification, while preserving full backward compatibility. Legacy names are marked with @deprecated JSDoc tags. Additionally, inline documentation is improved to clarify the mapping between internal field names (e.g., lepusCode, manifest) and their official spec equivalents (MTS/BTS).
Changes:
- New type/value aliases added for spec-aligned naming across multiple packages (
web-constants,template-webpack-plugin,web-core,web-core-wasm) @deprecatedJSDoc tags added to legacy types/classes; documentation updated to reference official Lynx terminology- Unrelated changes to
pnpm-workspace.yaml(adds non-existentpackages/repl) andpnpm-lock.yaml(bumps several packages to@rsbuild/core@2.0.0-beta.3)
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
pnpm-workspace.yaml |
Adds packages/repl entry (directory does not exist) |
pnpm-lock.yaml |
Several packages now resolve against @rsbuild/core@2.0.0-beta.3; unrelated to stated PR purpose |
packages/webpack/template-webpack-plugin/src/index.ts |
Exports new LynxBundlePlugin, LynxBundlePluginOptions, BundleHooks aliases |
packages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.ts |
Adds @deprecated to legacy types; introduces LynxBundlePlugin, BundleHooks, LynxBundlePluginOptions aliases; improves JSDoc |
packages/webpack/template-webpack-plugin/src/LynxEncodePlugin.ts |
Updates inline comment for app-service.js BTS entry point name |
packages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts |
Updates inline comment for app-service.js BTS entry point name |
packages/webpack/react-webpack-plugin/src/loaders/options.ts |
Adds clarifying comment for lepus path segment |
packages/webpack/react-webpack-plugin/src/ReactWebpackPlugin.ts |
Updates doc to use MTS/BTS terminology instead of Lepus/JS |
packages/web-platform/web-mainthread-apis/ts/crossThreadHandlers/registerCallLepusMethodHandler.ts |
Adds JSDoc explaining legacy "Lepus" term |
packages/web-platform/web-mainthread-apis/ts/createMainThreadGlobalThis.ts |
Adds field and function JSDoc for MTS terminology |
packages/web-platform/web-core/src/index.ts |
Re-exports LynxBundle from web-constants |
packages/web-platform/web-core-wasm/ts/types/DecodedTemplate.ts |
Marks DecodedTemplate deprecated; adds DecodedBundle alias |
packages/web-platform/web-core-wasm/ts/encode/webEncoder.ts |
Adds JSDoc for manifest/lepusCode fields |
packages/web-platform/web-core-wasm/ts/constants.ts |
Adds BundleSectionLabel constant with spec-aligned names |
packages/web-platform/web-constants/src/types/TemplateLoader.ts |
Marks TemplateLoader deprecated; adds BundleLoader alias |
packages/web-platform/web-constants/src/types/LynxModule.ts |
Marks LynxTemplate deprecated; adds LynxBundle alias; improves field JSDoc |
packages/web-platform/web-constants/src/endpoints.ts |
Adds JSDoc to callLepusMethodEndpoint |
packages/rspeedy/plugin-react/src/pluginReactLynx.ts |
Updates option JSDoc to use MTS/BTS terminology |
packages/rspeedy/plugin-react/src/index.ts |
Exports LynxBundlePlugin and BundleHooks aliases alongside deprecated legacy exports |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
packages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.ts:381
- The
@deprecatedannotation on theLynxTemplatePluginclass is misleading in this refactoring. SinceLynxBundlePluginis defined asconst LynxBundlePlugin = LynxTemplatePlugin, TypeScript will infer its type astypeof LynxTemplatePlugin. BecauseLynxTemplatePluginis marked@deprecated, IDEs (e.g. VS Code) will still display a strikethrough or deprecation warning when code hovers overLynxBundlePlugin, because the inferred type references the deprecated symbol. This undermines the purpose of providing a clean non-deprecated alias.
The standard pattern for this kind of rename-with-backward-compat is:
- Keep the canonical (new) name as the primary declaration (no
@deprecated) - Create the old name as a deprecated type alias that references the new canonical name
Since renaming the actual class is a larger refactor, an intermediate approach would be to avoid placing the @deprecated tag on LynxTemplatePlugin itself and instead only note in its JSDoc that it's a legacy name, pointing to LynxBundlePlugin as the preferred name — but without the @deprecated tag triggering IDE warnings for the new alias.
* @deprecated Use {@link LynxBundlePlugin} instead. This alias is retained for
* backward compatibility.
*
* @public
*/
export class LynxTemplatePlugin {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * @deprecated The name `OriginManifest` is a legacy artifact. "Manifest" here | ||
| * refers to Background Thread Script (BTS) chunks in the official Lynx | ||
| * specification. |
There was a problem hiding this comment.
The @deprecated annotation on OriginManifest does not provide a {@link} reference to a replacement type. Unlike the other deprecated types in this PR (which all have @deprecated Use {@link NewName} instead), this one leaves users without a clear alternative to migrate to. Either add a {@link} to a replacement type alias (e.g. if one should be introduced), or include explicit guidance in the deprecation text (e.g. Use the inline record type directly or see {@link EncodeOptions.manifest}).
| * specification. | |
| * specification. Use the inline record type directly or see | |
| * {@link EncodeOptions.manifest}. |
pnpm-lock.yaml
Outdated
| @@ -941,7 +941,7 @@ importers: | |||
| version: 1.1.1 | |||
| rsbuild-plugin-tailwindcss: | |||
| specifier: 0.2.4 | |||
| version: 0.2.4(@rsbuild/core@1.7.3)(tailwindcss@3.4.19) | |||
| version: 0.2.4(@rsbuild/core@2.0.0-beta.3(core-js@3.48.0))(tailwindcss@3.4.19) | |||
There was a problem hiding this comment.
The pnpm-lock.yaml changes include several packages now resolving with @rsbuild/core@2.0.0-beta.3 (a beta release) instead of the catalog-pinned stable version 1.7.3 for importers like packages/rspeedy/upgrade-rspeedy and website. This is a significant unintended side-effect that is completely unrelated to the API alias/terminology changes described in this PR, and it may introduce instability. These lock file changes should be investigated and separated from this PR.
pnpm-workspace.yaml
Outdated
| - packages/background-only | ||
| - packages/lynx/* | ||
| - packages/mcp-servers/* | ||
| - packages/repl |
There was a problem hiding this comment.
The pnpm-workspace.yaml diff adds packages/repl to the workspace, but this directory does not exist in the repository. This will cause pnpm install to fail (or at least produce a warning) because there is no package.json to resolve under packages/repl. This change appears completely unrelated to the stated purpose of this PR (adding API aliases for the Lynx specification terminology) and should be removed or addressed in a separate PR.
| - packages/repl |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts (1)
71-71: Documentation clarity: consider expanding "BTS" abbreviation.The comment update aligns well with the PR's terminology standardization goals. The abbreviation "BTS" (Background Thread Script) is concise but might benefit from being expanded on first use for improved clarity, especially for developers unfamiliar with the official specification.
📝 Optional expansion for clarity
- // `/app-service.js` is the conventional BTS entry point in a Lynx Bundle. + // `/app-service.js` is the conventional BTS (Background Thread Script) entry point in a Lynx Bundle.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts` at line 71, The inline comment in WebEncodePlugin referencing "`/app-service.js` is the conventional BTS entry point in a Lynx Bundle." should expand the abbreviation on first use for clarity; update that comment in WebEncodePlugin.ts to read something like "BTS (Background Thread Script)" on first mention (e.g., "`/app-service.js` is the conventional BTS (Background Thread Script) entry point in a Lynx Bundle") so unfamiliar readers immediately see the full term while keeping subsequent mentions as "BTS".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.ts`:
- Line 1119: The exported alias LynxBundlePlugin lacks an explicit type
annotation which breaks declaration generation under --isolatedDeclarations;
update the export so LynxBundlePlugin is declared with an explicit type (e.g.,
annotate it as the same type as LynxTemplatePlugin using a typeof or the
plugin's exported type/interface) so the compiler can emit declarations without
analyzing other modules — locate the export "export const LynxBundlePlugin =
LynxTemplatePlugin;" and change it to include the explicit type annotation for
LynxBundlePlugin.
---
Nitpick comments:
In `@packages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts`:
- Line 71: The inline comment in WebEncodePlugin referencing "`/app-service.js`
is the conventional BTS entry point in a Lynx Bundle." should expand the
abbreviation on first use for clarity; update that comment in WebEncodePlugin.ts
to read something like "BTS (Background Thread Script)" on first mention (e.g.,
"`/app-service.js` is the conventional BTS (Background Thread Script) entry
point in a Lynx Bundle") so unfamiliar readers immediately see the full term
while keeping subsequent mentions as "BTS".
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
packages/rspeedy/plugin-react/src/index.tspackages/rspeedy/plugin-react/src/pluginReactLynx.tspackages/web-platform/web-constants/src/endpoints.tspackages/web-platform/web-constants/src/types/LynxModule.tspackages/web-platform/web-constants/src/types/TemplateLoader.tspackages/web-platform/web-core-wasm/ts/constants.tspackages/web-platform/web-core-wasm/ts/encode/webEncoder.tspackages/web-platform/web-core-wasm/ts/types/DecodedTemplate.tspackages/web-platform/web-core/src/index.tspackages/web-platform/web-mainthread-apis/ts/createMainThreadGlobalThis.tspackages/web-platform/web-mainthread-apis/ts/crossThreadHandlers/registerCallLepusMethodHandler.tspackages/webpack/react-webpack-plugin/src/ReactWebpackPlugin.tspackages/webpack/react-webpack-plugin/src/loaders/options.tspackages/webpack/template-webpack-plugin/src/LynxEncodePlugin.tspackages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.tspackages/webpack/template-webpack-plugin/src/WebEncodePlugin.tspackages/webpack/template-webpack-plugin/src/index.tspnpm-workspace.yaml
packages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.ts (1)
1113-1113:⚠️ Potential issue | 🔴 CriticalAdd explicit type annotation for exported alias (build blocker).
Line 1113 currently fails declaration emit with
--isolatedDeclarationsbecause the exportedconstalias has no explicit type.Suggested fix
-export const LynxBundlePlugin = LynxTemplatePlugin; +export const LynxBundlePlugin: typeof LynxTemplatePlugin = + LynxTemplatePlugin;As per coding guidelines
**/*.{ts,tsx}: Use TypeScript with the strictest mode configuration as defined in tsconfig.json.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.ts` at line 1113, The exported alias LynxBundlePlugin lacks an explicit type which breaks declaration emit under --isolatedDeclarations; fix by adding an explicit type annotation to the exported const (make LynxBundlePlugin have the same type as LynxTemplatePlugin, e.g. annotate it with typeof LynxTemplatePlugin) so the declaration file can be emitted correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.ts`:
- Around line 1109-1113: The export for LynxBundlePlugin currently carries a
contradictory `@deprecated` JSDoc; remove the deprecation annotation/comment from
the declaration of export const LynxBundlePlugin = LynxTemplatePlugin so that
LynxBundlePlugin remains the canonical alias, leaving the deprecation on
LynxTemplatePlugin intact; update or simplify the JSDoc above LynxBundlePlugin
to a short non-deprecated note (or remove it entirely) to avoid inverted
migration warnings in IDEs.
---
Duplicate comments:
In `@packages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.ts`:
- Line 1113: The exported alias LynxBundlePlugin lacks an explicit type which
breaks declaration emit under --isolatedDeclarations; fix by adding an explicit
type annotation to the exported const (make LynxBundlePlugin have the same type
as LynxTemplatePlugin, e.g. annotate it with typeof LynxTemplatePlugin) so the
declaration file can be emitted correctly.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/rspeedy/plugin-react/src/index.tspackages/web-platform/web-constants/src/types/LynxModule.tspackages/web-platform/web-constants/src/types/TemplateLoader.tspackages/web-platform/web-core-wasm/ts/constants.tspackages/web-platform/web-core-wasm/ts/types/DecodedTemplate.tspackages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/web-platform/web-core-wasm/ts/constants.ts
- packages/rspeedy/plugin-react/src/index.ts
packages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.ts
Outdated
Show resolved
Hide resolved
Merging this PR will improve performance by 36.97%
Performance Changes
Comparing Footnotes
|
Web Explorer#8003 Bundle Size — 384.21KiB (0%).9e46237(current) vs 7518b72 main#7993(baseline) Bundle metrics
Bundle size by type
|
| Current #8003 |
Baseline #7993 |
|
|---|---|---|
253.26KiB |
253.26KiB |
|
95.85KiB |
95.85KiB |
|
35.1KiB |
35.1KiB |
Bundle analysis report Branch Huxpro/fetch-lynx-docs-v1 Project dashboard
Generated by RelativeCI Documentation Report issue
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/rspeedy/plugin-react/etc/react-rsbuild-plugin.api.md`:
- Around line 54-56: The exported interface LynxBundlePlugin is missing JSDoc
with a release tag; add a JSDoc block immediately above the LynxBundlePlugin
declaration (matching the style used for PluginReactLynxOptions) that includes a
short one-line description and the `@public` release tag so API Extractor no
longer marks it as undocumented.
In `@packages/webpack/template-webpack-plugin/etc/template-webpack-plugin.api.md`:
- Around line 79-82: Edit the JSDoc for the exported constant LynxBundlePlugin
in LynxTemplatePlugin.ts: remove the `@deprecated` tag and ensure it is annotated
`@public` (leave no deprecation text), while keeping the LynxTemplatePlugin class
marked `@deprecated` and pointing to LynxBundlePlugin; after updating the JSDoc,
rebuild/regenerate the API report by running pnpm turbo api-extractor -- --local
so the packages/webpack/template-webpack-plugin API file reflects the change.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/rspeedy/plugin-react/etc/react-rsbuild-plugin.api.mdpackages/rspeedy/plugin-react/src/index.tspackages/webpack/template-webpack-plugin/etc/template-webpack-plugin.api.md
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/rspeedy/plugin-react/src/index.ts
packages/webpack/template-webpack-plugin/etc/template-webpack-plugin.api.md
Outdated
Show resolved
Hide resolved
Add official API aliases (LynxBundle, BundleLoader, LynxBundlePlugin, BundleHooks, LynxBundlePluginOptions, DecodedBundle, BundleSectionLabel) for legacy internal names (LynxTemplate, TemplateLoader, LynxTemplatePlugin, TemplateHooks, LynxTemplatePluginOptions, DecodedTemplate, TemplateSectionLabel). Mark legacy names with @deprecated to guide users to official terminology while maintaining full backward compatibility. Add comprehensive JSDoc clarifications on definition sites explaining the mapping between internal field names and official spec terms, plus wire format constraints on hardcoded field names. Remove redundant call-site comments since IDE will show @deprecated warnings.
The new canonical names (LynxBundle, BundleLoader, DecodedBundle, BundleSectionLabel, BundleHooks, LynxBundlePluginOptions, LynxBundlePlugin) are now the actual interface/const definitions, and the old names (LynxTemplate, TemplateLoader, DecodedTemplate, TemplateSectionLabel, TemplateHooks, LynxTemplatePluginOptions, LynxTemplatePlugin) are @deprecated aliases pointing to them.
… lockfile drift - Add `typeof LynxTemplatePlugin` annotation on LynxBundlePlugin export to satisfy --isolatedDeclarations (TS9010). - Revert accidental pnpm-lock.yaml and pnpm-workspace.yaml changes that were not part of the terminology refactoring.
- Fix sort-imports ESLint error in plugin-react/src/index.ts - Update api-extractor report for @lynx-js/react-rsbuild-plugin - Update api-extractor report for @lynx-js/template-webpack-plugin
adada26 to
2f7528a
Compare
Co-authored-by: Haoyang Wang <12288479+PupilTong@users.noreply.github.com> Signed-off-by: Xuan Huang (黄玄) <5563315+Huxpro@users.noreply.github.com>
- Remove contradictory @deprecated on LynxBundlePlugin export (CodeRabbit) - Add @public JSDoc to LynxBundlePlugin interface in plugin-react (CodeRabbit) - Add {@link EncodeOptions.manifest} guidance to OriginManifest @deprecated (Copilot) - Replace TemplateSectionLabel with BundleSectionLabel directly in web-core-wasm (PupilTong) - Add LynxBundlePlugin symbol expose alongside LynxTemplatePlugin (colinaaa) - Regenerate API reports
…ugin Update the exposed plugin API object key and all consumers (pluginLynxConfig, tests) to use LynxBundlePlugin consistently.
colinaaa
left a comment
There was a problem hiding this comment.
It's fine to make the change in packages/rspeedy/plugin-config in the next PR :)
| { LynxBundlePlugin: typeof LynxTemplatePlugin } | ||
| >( | ||
| Symbol.for('LynxTemplatePlugin'), | ||
| Symbol.for('LynxBundlePlugin'), | ||
| ) |
There was a problem hiding this comment.
We should be compatible with the legacy @lynx-js/react-rsbuild-plugin.
const exposed = api.useExposed<
{ LynxBundlePlugin: typeof LynxTemplatePlugin }
>(
Symbol.for('LynxBundlePlugin'),
) ?? api.useExposed<
{ LynxTemplatePlugin: typeof LynxTemplatePlugin }
>(
Symbol.for('LynxTemplatePlugin'),
)| } | ||
|
|
||
| const { LynxTemplatePlugin: LynxTemplatePluginClass } = exposed | ||
| const { LynxBundlePlugin: LynxTemplatePluginClass } = exposed |
| { LynxBundlePlugin: typeof LynxTemplatePlugin } | ||
| >( | ||
| Symbol.for('LynxTemplatePlugin'), | ||
| Symbol.for('LynxBundlePlugin'), |
There was a problem hiding this comment.
There should also be a changeset for @lynx-js/config-rsbuild-plugin.
| "@lynx-js/web-constants": patch | ||
| "@lynx-js/web-core": patch | ||
| "@lynx-js/react-webpack-plugin": patch | ||
| "@lynx-js/template-webpack-plugin": patch |
There was a problem hiding this comment.
Shall we also rename @lynx-js/template-webpack-plugin to @lynx-js/bundle-webpack-plugin? (Might be a breaking change)
There was a problem hiding this comment.
+1. @lynx-js/template-webpack-plugin should only be used by us.
| api.modifyBundlerChain(chain => { | ||
| const exposed = api.useExposed< | ||
| { LynxTemplatePlugin: typeof LynxTemplatePlugin } | ||
| { LynxBundlePlugin: typeof LynxTemplatePlugin } |
There was a problem hiding this comment.
Do we need to migrate LynxTemplatePlugin export in @lynx-js/bundle-webpack-plugin to LynxBundlePlugin too? (Or use a fallback LynxBundlePlugin ?? LynxTemplatePlugin now and migrate to LynxBundlePlugin future)
| * | ||
| * @public | ||
| */ | ||
| export class LynxTemplatePlugin { |
There was a problem hiding this comment.
I prefer to rename it to LynxBundlePlugin at this point, and then add export const LynxTemplatePlugin: typeof LynxBundlePlugin = LynxBundlePlugin; below.
Summary
Add official API aliases (LynxBundle, BundleLoader, LynxBundlePlugin, BundleHooks, LynxBundlePluginOptions, DecodedBundle, BundleSectionLabel) for legacy internal names to align with the official lynxjs.org specification. Mark deprecated names with @deprecated JSDoc tags to guide users to official terminology while maintaining full backward compatibility.
Add comprehensive JSDoc clarifications explaining the mapping between internal field names and official spec terms, plus wire format constraints. Remove redundant call-site comments since IDEs will automatically show @deprecated warnings.
Affected npm packages
Summary by CodeRabbit
Refactor
New Features
Documentation
Tests
Checklist