feat(autolink): add autolink codegen package#2601
Merged
jianliang00 merged 1 commit intoMay 12, 2026
Merged
Conversation
🦋 Changeset detectedLatest commit: c932220 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 |
Contributor
|
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 adds ChangesNative Autolink Codegen Package
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
jianliang00
force-pushed
the
p/wangjianliang/autolink-codegen-package
branch
from
23515a8 to
e5ce6f7
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/lynx/autolink-codegen/package.json`:
- Around line 1-40: Add a top-level "packageManager" field to this package.json
for `@lynx-js/autolink-codegen` using the repo's standard pnpm version string
(e.g. "pnpm@<repo-pnpm-version>"); place it alongside the existing top-level
fields (near "engines" or "type") so installs use a consistent pnpm across the
workspace and satisfy the workspace package policy.
In `@packages/lynx/autolink-codegen/src/index.ts`:
- Around line 338-345: The parser currently allows parameters whose parsed type
is void, which later breaks emitters (e.g., toObjCParamType throws and
toJavaType produces invalid signatures); update parseParams so that after
calling parseType (the same call shown in the returned object creation) it
checks the resolved type for a void-kind and rejects it by throwing a clear
error that includes the parameter name and context (use the same
`${moduleName}.${methodName}.${name}` context string). Ensure the validation
references parseType's result and is applied for each parameter before returning
from parseParams so void parameters are rejected at parse time rather than
during codegen.
In `@packages/lynx/autolink-codegen/test/codegen.test.ts`:
- Around line 107-112: The test currently asserts exact ordering of generated
file paths by calling expect(files.map((file) => file.path)).toEqual([...]),
which is order-sensitive; update the assertion to be order-insensitive by either
sorting the array returned by files.map((file) => file.path) before comparing or
using an order-insensitive matcher (e.g.,
expect(...).toEqual(expect.arrayContaining(...)) and also assert lengths).
Locate the assertion in packages/lynx/autolink-codegen/test/codegen.test.ts (the
files variable and the map callback) and change the assertion to compare sorted
arrays or use arrayContaining with a length check so generate() can return paths
in any order.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 79001243-7113-4631-8db2-cdfb0a259ac8
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
.changeset/native-autolink-codegen.mdpackages/lynx/autolink-codegen/CHANGELOG.mdpackages/lynx/autolink-codegen/README.mdpackages/lynx/autolink-codegen/package.jsonpackages/lynx/autolink-codegen/rslib.config.tspackages/lynx/autolink-codegen/src/cli.tspackages/lynx/autolink-codegen/src/index.tspackages/lynx/autolink-codegen/test/codegen.test.tspackages/lynx/autolink-codegen/tsconfig.build.jsonpackages/lynx/autolink-codegen/tsconfig.jsonpackages/lynx/autolink-codegen/tsconfig.test.jsonpackages/lynx/autolink-codegen/turbo.jsonpackages/lynx/autolink-codegen/vitest.config.tspackages/lynx/tsconfig.jsonvitest.config.ts
Contributor
There was a problem hiding this comment.
Pull request overview
Adds the new @lynx-js/autolink-codegen package (split out of #2587) and wires it into the monorepo so it can be built/tested as part of the workspace. The package provides a Native-only code generator (TS facade + Android + iOS specs) with a CLI and Vitest coverage.
Changes:
- Introduce
packages/lynx/autolink-codegenwith codegen implementation, CLI entrypoint, and Vitest tests. - Register the package in workspace tooling (Vitest projects glob +
packages/lynxTS project references) and lockfile. - Add a Changeset + package README/CHANGELOG metadata.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.ts | Expands Lynx Vitest workspace discovery to include all packages/lynx/*/vitest.config.ts. |
| pnpm-lock.yaml | Adds the new workspace importer for packages/lynx/autolink-codegen. |
| packages/lynx/tsconfig.json | Adds TS project reference for the new package build config. |
| packages/lynx/autolink-codegen/vitest.config.ts | Defines Vitest configuration for the new package tests. |
| packages/lynx/autolink-codegen/turbo.json | Declares Turbo tasks/outputs for build and test in the new package. |
| packages/lynx/autolink-codegen/tsconfig.test.json | Adds TS config for typechecking tests. |
| packages/lynx/autolink-codegen/tsconfig.json | Adds project references for build + test TS configs. |
| packages/lynx/autolink-codegen/tsconfig.build.json | Adds TS build config for the package source. |
| packages/lynx/autolink-codegen/test/codegen.test.ts | Adds unit tests for parsing, generation, error cases, and path safety. |
| packages/lynx/autolink-codegen/src/index.ts | Implements parsing + generation + filesystem writing for JS/Android/iOS specs. |
| packages/lynx/autolink-codegen/src/cli.ts | Adds the lynx-autolink-codegen CLI wrapper around runCodegen. |
| packages/lynx/autolink-codegen/rslib.config.ts | Configures rslib build entries for library + CLI bundles. |
| packages/lynx/autolink-codegen/README.md | Documents usage, inputs (types/**/*.d.ts, lynx.ext.json), and outputs. |
| packages/lynx/autolink-codegen/package.json | Declares package metadata, exports, and CLI binary mapping. |
| packages/lynx/autolink-codegen/CHANGELOG.md | Introduces initial changelog entry for the new package. |
| .changeset/native-autolink-codegen.md | Adds changeset to release the new package as a minor bump. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Merging this PR will degrade performance by 16.12%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | 008-many-use-state-destroyBackground |
8 ms | 9.5 ms | -16.12% |
Comparing jianliang00:p/wangjianliang/autolink-codegen-package (c932220) with main (1e1257e)2
Footnotes
-
26 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
-
No successful run was found on
main(47bb2ff) during the generation of this report, so 1e1257e was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
Web Explorer#9670 Bundle Size — 901.38KiB (0%).c932220(current) vs f8a3cbd main#9669(baseline) Bundle metrics
|
| Current #9670 |
Baseline #9669 |
|
|---|---|---|
 Initial JS |
45.06KiB |
45.06KiB |
| Initial CSS |
2.22KiB |
2.22KiB |
 Cache Invalidation |
0% |
1.53% |
| Chunks |
9 |
9 |
| Assets |
11 |
11 |
| Modules |
230(+0.44%) |
229 |
| Duplicate Modules |
11 |
11 |
| Duplicate Code |
27.21%(-0.04%) |
27.22% |
| Packages |
10 |
10 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #9670 |
Baseline #9669 |
|
|---|---|---|
| JS |
497.1KiB |
497.1KiB |
| Other |
402.06KiB |
402.06KiB |
| CSS |
2.22KiB |
2.22KiB |
Bundle analysis report Branch jianliang00:p/wangjianliang/auto... Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#1228 Bundle Size — 207.46KiB (0%).c932220(current) vs f8a3cbd main#1227(baseline) Bundle metrics
|
| Current #1228 |
Baseline #1227 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
192 |
192 |
| Duplicate Modules |
77 |
77 |
| Duplicate Code |
44.38% |
44.38% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #1228 |
Baseline #1227 |
|
|---|---|---|
| IMG |
111.23KiB |
111.23KiB |
| Other |
96.23KiB |
96.23KiB |
Bundle analysis report Branch jianliang00:p/wangjianliang/auto... Project dashboard
Generated by RelativeCI Documentation Report issue
React External#1210 Bundle Size — 693.04KiB (0%).c932220(current) vs f8a3cbd main#1209(baseline) Bundle metrics
|
| Current #1210 |
Baseline #1209 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
17 |
17 |
| Duplicate Modules |
5 |
5 |
| Duplicate Code |
8.59% |
8.59% |
| Packages |
0 |
0 |
| Duplicate Packages |
0 |
0 |
Bundle analysis report Branch jianliang00:p/wangjianliang/auto... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#8096 Bundle Size — 236.51KiB (0%).c932220(current) vs f8a3cbd main#8095(baseline) Bundle metrics
|
| Current #8096 |
Baseline #8095 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
197 |
197 |
| Duplicate Modules |
80 |
80 |
| Duplicate Code |
44.87% |
44.87% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #8096 |
Baseline #8095 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
90.75KiB |
90.75KiB |
Bundle analysis report Branch jianliang00:p/wangjianliang/auto... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example with Element Template#363 Bundle Size — 197.79KiB (0%).c932220(current) vs f8a3cbd main#362(baseline) Bundle metrics
|
| Current #363 |
Baseline #362 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
79(-2.47%) |
81 |
| Duplicate Modules |
23 |
23 |
| Duplicate Code |
40.32%(+0.07%) |
40.29% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #363 |
Baseline #362 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
52.03KiB |
52.03KiB |
Bundle analysis report Branch jianliang00:p/wangjianliang/auto... Project dashboard
Generated by RelativeCI Documentation Report issue
jianliang00
force-pushed
the
p/wangjianliang/autolink-codegen-package
branch
2 times, most recently
from
46e94a5 to
3f96789
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/lynx/autolink-codegen/src/index.ts`:
- Around line 684-689: The packageName read via readRequiredString is not
validated as a Java package identifier; add a validation step after the current
read (for the packageName variable) that enforces Java package-name rules
(dot-separated identifiers, each starting with a letter/underscore, containing
only letters/digits/underscores, no empty segments) — implement a helper like
isValidJavaPackageName(packageName) and if invalid throw a clear manifest reader
error mentioning platforms.android.packageName; integrate this check before any
use (e.g., before the generated package declaration "package
${packageName}.generated;" and before the code that splits on '.' to build
output directories) so malformed names fail fast with a helpful message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3cac2c89-b28c-4c37-ae14-3ce7ab501d8a
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
.changeset/native-autolink-codegen.mdpackages/lynx/autolink-codegen/CHANGELOG.mdpackages/lynx/autolink-codegen/README.mdpackages/lynx/autolink-codegen/package.jsonpackages/lynx/autolink-codegen/rslib.config.tspackages/lynx/autolink-codegen/src/cli.tspackages/lynx/autolink-codegen/src/index.tspackages/lynx/autolink-codegen/test/codegen.test.tspackages/lynx/autolink-codegen/tsconfig.build.jsonpackages/lynx/autolink-codegen/tsconfig.jsonpackages/lynx/autolink-codegen/tsconfig.test.jsonpackages/lynx/autolink-codegen/turbo.jsonpackages/lynx/autolink-codegen/vitest.config.tspackages/lynx/tsconfig.jsonvitest.config.ts
✅ Files skipped from review due to trivial changes (7)
- packages/lynx/autolink-codegen/CHANGELOG.md
- packages/lynx/autolink-codegen/README.md
- packages/lynx/autolink-codegen/tsconfig.json
- .changeset/native-autolink-codegen.md
- packages/lynx/autolink-codegen/tsconfig.build.json
- vitest.config.ts
- packages/lynx/autolink-codegen/tsconfig.test.json
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/lynx/tsconfig.json
- packages/lynx/autolink-codegen/vitest.config.ts
- packages/lynx/autolink-codegen/package.json
- packages/lynx/autolink-codegen/src/cli.ts
- packages/lynx/autolink-codegen/turbo.json
- packages/lynx/autolink-codegen/rslib.config.ts
- packages/lynx/autolink-codegen/test/codegen.test.ts
jianliang00
force-pushed
the
p/wangjianliang/autolink-codegen-package
branch
from
3f96789 to
aa798a9
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/lynx/autolink-codegen/src/index.ts (1)
880-884: 💤 Low valueVerify the generated
.mshim is intentional — it contains no@implementation.
generateIosImplementationemits an Objective-C source file whose only content is#import "${module.name}Spec.h". Since${module.name}Spec.hdeclares a@protocol, this.mproduces an empty translation unit — it has no class/category/protocol implementation, so the file contributes nothing at link time but still adds to build inputs. If the intent is a stub for user-provided extension code, consider either:
- skipping
.mgeneration entirely (protocol declarations live in the header), or- emitting an
@implementationstub or a clear comment indicating that consumers must provide their own implementation conforming to the spec.Could you confirm whether downstream Lynx iOS tooling (CocoaPods spec, SwiftPM, or the Xcode build) expects this
.mfile to exist? If yes, a short justification comment in the generator would help future maintainers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/lynx/autolink-codegen/src/index.ts` around lines 880 - 884, The generated .m currently only imports "${module.name}Spec.h" (see generateIosImplementation) producing an empty translation unit; update the generator to either stop emitting the .m at all when no implementation is needed, or emit a clear stub (an `@implementation` block or an explanatory comment) so the file has intent — choose one approach and apply it consistently for generateIosImplementation: if you keep the file, add a concise justification comment at the top and an empty `@implementation/`${module.name}Spec placeholder; if you remove it, ensure callers of generateIosImplementation handle the absence of an Objective-C implementation file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/lynx/autolink-codegen/src/index.ts`:
- Around line 268-279: The loop in runCodegen writes files as it resolves each
target, so a later resolveInside failure can leave partially written output;
change runCodegen to first iterate over the generated files from generate({ root
}) and call resolveInside for every file.path (using the same 'package root'
context) to produce and validate all target paths up front, collecting the
resolved targets into an array, and only after all resolves succeed perform the
mkdirSync+writeFileSync phase using the already-resolved targets; keep function
names generate, resolveInside and runCodegen to locate the change.
---
Nitpick comments:
In `@packages/lynx/autolink-codegen/src/index.ts`:
- Around line 880-884: The generated .m currently only imports
"${module.name}Spec.h" (see generateIosImplementation) producing an empty
translation unit; update the generator to either stop emitting the .m at all
when no implementation is needed, or emit a clear stub (an `@implementation` block
or an explanatory comment) so the file has intent — choose one approach and
apply it consistently for generateIosImplementation: if you keep the file, add a
concise justification comment at the top and an empty
`@implementation/`${module.name}Spec placeholder; if you remove it, ensure callers
of generateIosImplementation handle the absence of an Objective-C implementation
file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e52d17dc-15d8-490a-9f2c-9566dfba5644
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
.changeset/native-autolink-codegen.mdpackages/lynx/autolink-codegen/CHANGELOG.mdpackages/lynx/autolink-codegen/README.mdpackages/lynx/autolink-codegen/package.jsonpackages/lynx/autolink-codegen/rslib.config.tspackages/lynx/autolink-codegen/src/cli.tspackages/lynx/autolink-codegen/src/index.tspackages/lynx/autolink-codegen/test/codegen.test.tspackages/lynx/autolink-codegen/tsconfig.build.jsonpackages/lynx/autolink-codegen/tsconfig.jsonpackages/lynx/autolink-codegen/tsconfig.test.jsonpackages/lynx/autolink-codegen/turbo.jsonpackages/lynx/autolink-codegen/vitest.config.tspackages/lynx/tsconfig.jsonvitest.config.ts
✅ Files skipped from review due to trivial changes (8)
- packages/lynx/autolink-codegen/turbo.json
- .changeset/native-autolink-codegen.md
- packages/lynx/autolink-codegen/CHANGELOG.md
- vitest.config.ts
- packages/lynx/autolink-codegen/vitest.config.ts
- packages/lynx/autolink-codegen/README.md
- packages/lynx/tsconfig.json
- packages/lynx/autolink-codegen/tsconfig.test.json
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/lynx/autolink-codegen/tsconfig.build.json
- packages/lynx/autolink-codegen/tsconfig.json
- packages/lynx/autolink-codegen/rslib.config.ts
- packages/lynx/autolink-codegen/package.json
- packages/lynx/autolink-codegen/src/cli.ts
jianliang00
force-pushed
the
p/wangjianliang/autolink-codegen-package
branch
from
aa798a9 to
725805b
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/lynx/autolink-codegen/test/codegen.test.ts (1)
140-155:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winResolve generated files by path before asserting content.
The path assertion is order-insensitive now, but Lines 140-155 still assume
files[0],files[1], andfiles[2]stay in a fixed order. A harmless emitter reordering would still make this test fail.Suggested test hardening
expect(files.map((file) => file.path).sort()).toEqual([ 'generated/StorageModule.ts', 'android/src/main/java/com/example/storage/generated/StorageModuleSpec.java', 'ios/src/generated/StorageModuleSpec.h', 'ios/src/generated/StorageModuleSpec.m', ].sort()); - expect(files[0]?.content).toContain('NativeModules.StorageModule'); - expect(files[1]?.content).toContain( + const filesByPath = new Map(files.map((file) => [file.path, file.content])); + expect(filesByPath.get('generated/StorageModule.ts')).toContain( + 'NativeModules.StorageModule', + ); + expect( + filesByPath.get( + 'android/src/main/java/com/example/storage/generated/StorageModuleSpec.java', + ), + ).toContain( 'package com.example.storage.generated;', ); - expect(files[1]?.content).toContain( + expect( + filesByPath.get( + 'android/src/main/java/com/example/storage/generated/StorageModuleSpec.java', + ), + ).toContain( 'import com.lynx.jsbridge.LynxContextModule;', ); - expect(files[1]?.content).toContain('import com.lynx.jsbridge.LynxMethod;'); - expect(files[1]?.content).toContain( + expect( + filesByPath.get( + 'android/src/main/java/com/example/storage/generated/StorageModuleSpec.java', + ), + ).toContain('import com.lynx.jsbridge.LynxMethod;'); + expect( + filesByPath.get( + 'android/src/main/java/com/example/storage/generated/StorageModuleSpec.java', + ), + ).toContain( 'import com.lynx.tasm.behavior.LynxContext;', ); - expect(files[1]?.content).toContain('public abstract void setValue'); - expect(files[2]?.content).toContain('@protocol StorageModuleSpec'); - expect(files[2]?.content).toContain( + expect( + filesByPath.get( + 'android/src/main/java/com/example/storage/generated/StorageModuleSpec.java', + ), + ).toContain('public abstract void setValue'); + expect(filesByPath.get('ios/src/generated/StorageModuleSpec.h')).toContain( + '@protocol StorageModuleSpec', + ); + expect(filesByPath.get('ios/src/generated/StorageModuleSpec.h')).toContain( '- (nullable NSString *)getValue:(NSString *)key;', );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/lynx/autolink-codegen/test/codegen.test.ts` around lines 140 - 155, The test assumes a stable order of the generated files (using files[0], files[1], files[2]) which is brittle; change the assertions to resolve files by their path/name before checking content (e.g., build a lookup from files[].path to file and use that lookup for expectations). Update the assertions that reference files[0], files[1], files[2] to instead find the correct file by path (or sort by path) and then assert on that file's .content; keep the existing assertion strings (e.g., 'NativeModules.StorageModule', 'package com.example.storage.generated;', '@protocol StorageModuleSpec') and use the lookup when calling expect(...).toContain(...).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/lynx/autolink-codegen/src/index.ts`:
- Around line 272-275: resolveInside only validates the lexical path; to prevent
escaping the package root via existing symlinks you must add a helper (e.g.,
assertNoSymlinkTraversal(root, target)) and call it for each computed target
before fs.mkdirSync and fs.writeFileSync. The helper should resolve root,
iterate path.relative(root, target) segments, join them incrementally, and throw
if fs.existsSync(current) && fs.lstatSync(current).isSymbolicLink() so any
symlink on the path from root to target is rejected; invoke this check in the
loop that computes target right before creating directories or writing files
(references: resolveInside, target, assertNoSymlinkTraversal).
- Around line 514-522: The parameter parser in the code that does
trimmed.split(',').map(...) for NativeModuleParam throws on trailing commas
because split produces an empty string segment; update the parsing pipeline (the
trimmed.split(',') usage inside this block that constructs NativeModuleParam for
moduleName/methodName in index.ts) to filter out empty or whitespace-only
segments (e.g., filter before map) so trailing commas are ignored and only
non-empty normalizedParam values are validated and mapped.
---
Duplicate comments:
In `@packages/lynx/autolink-codegen/test/codegen.test.ts`:
- Around line 140-155: The test assumes a stable order of the generated files
(using files[0], files[1], files[2]) which is brittle; change the assertions to
resolve files by their path/name before checking content (e.g., build a lookup
from files[].path to file and use that lookup for expectations). Update the
assertions that reference files[0], files[1], files[2] to instead find the
correct file by path (or sort by path) and then assert on that file's .content;
keep the existing assertion strings (e.g., 'NativeModules.StorageModule',
'package com.example.storage.generated;', '@protocol StorageModuleSpec') and use
the lookup when calling expect(...).toContain(...).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 102362de-1794-4aa9-9bb0-a20ea9f22f97
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
.changeset/native-autolink-codegen.mdpackages/lynx/autolink-codegen/CHANGELOG.mdpackages/lynx/autolink-codegen/README.mdpackages/lynx/autolink-codegen/package.jsonpackages/lynx/autolink-codegen/rslib.config.tspackages/lynx/autolink-codegen/src/cli.tspackages/lynx/autolink-codegen/src/index.tspackages/lynx/autolink-codegen/test/codegen.test.tspackages/lynx/autolink-codegen/tsconfig.build.jsonpackages/lynx/autolink-codegen/tsconfig.jsonpackages/lynx/autolink-codegen/tsconfig.test.jsonpackages/lynx/autolink-codegen/turbo.jsonpackages/lynx/autolink-codegen/vitest.config.tspackages/lynx/tsconfig.jsonvitest.config.ts
✅ Files skipped from review due to trivial changes (9)
- packages/lynx/autolink-codegen/CHANGELOG.md
- packages/lynx/autolink-codegen/README.md
- packages/lynx/autolink-codegen/tsconfig.test.json
- packages/lynx/tsconfig.json
- packages/lynx/autolink-codegen/turbo.json
- packages/lynx/autolink-codegen/package.json
- packages/lynx/autolink-codegen/vitest.config.ts
- packages/lynx/autolink-codegen/tsconfig.json
- .changeset/native-autolink-codegen.md
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/lynx/autolink-codegen/tsconfig.build.json
- vitest.config.ts
- packages/lynx/autolink-codegen/src/cli.ts
- packages/lynx/autolink-codegen/rslib.config.ts
jianliang00
force-pushed
the
p/wangjianliang/autolink-codegen-package
branch
from
725805b to
79f0b68
Compare
jianliang00
force-pushed
the
p/wangjianliang/autolink-codegen-package
branch
from
79f0b68 to
9df60d7
Compare
jianliang00
force-pushed
the
p/wangjianliang/autolink-codegen-package
branch
from
9df60d7 to
c932220
Compare
PupilTong
approved these changes
May 12, 2026
PupilTong
approved these changes
May 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
Summary by CodeRabbit