refactor: replace rollup-plugin-license with native Rolldown plugin#81
refactor: replace rollup-plugin-license with native Rolldown plugin#81naitokosuke wants to merge 7 commits intounjs:mainfrom
rollup-plugin-license with native Rolldown plugin#81Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 5 minutes and 39 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRemoves Changes
Sequence DiagramsequenceDiagram
participant Rolldown as Rolldown Build
participant Plugin as License Plugin
participant FS as File System
participant Bundle as Output Bundle
Rolldown->>Plugin: generateBundle hook triggered
activate Plugin
Plugin->>Plugin: Scan module IDs for node_modules paths
Plugin->>Plugin: Collect & deduplicate package directories
loop For each dependency
Plugin->>FS: Read package.json
FS-->>Plugin: Package metadata
Plugin->>FS: Read LICENSE* / LICENCE* files
FS-->>Plugin: License text (if found)
end
Plugin->>Plugin: Group by identical license text, sort & consolidate
Plugin->>Plugin: Render markdown for THIRD-PARTY-LICENSES.md
Plugin->>Bundle: Write or append THIRD-PARTY-LICENSES.md
deactivate Plugin
Bundle-->>Rolldown: Bundle complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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 |
naitokosuke
changed the title
rollup-plugin-license with native Rolldown plugin
|
Thanks for PR 🙏🏼 Once you could confirm it is valid i can take a look and locally test before release 👍🏼 |
Adds a characterization test that verifies THIRD-PARTY-LICENSES.md output using a snapshot, ensuring behavioral equivalence when replacing the license collection implementation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
naitokosuke
force-pushed
the
rm-rollup-plugin-license
branch
from
a546cb3 to
b2e2d11
Compare
|
@pi0 I did a self-review and made some tweaks. I'd like to add a test to ensure the license output doesn't change after this refactor, but I'm not sure about the best approach. I considered snapshot testing, but the current fixture depends on defu just for this purpose, which feels like unnecessary coupling. Do you have any suggestions for a better way to verify output parity? I've also verified locally that building |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
test/fixture/package.json (1)
17-18: Pindefuto an exact version in the fixture.
"*"makes the bundled chunk list andTHIRD-PARTY-LICENSES.mdsnapshot depend on whatever release installs that day. An exact version keeps this characterization test reproducible.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/fixture/package.json` around lines 17 - 18, In test/fixture/package.json replace the wildcard devDependency for "defu" ("*") with an exact semver string matching the version recorded in your lockfile or currently installed in node_modules (so the fixture is reproducible); update the "defu" entry under devDependencies to that exact version and re-run the snapshot/packaging tests to verify the THIRD-PARTY-LICENSES.md and bundled chunk lists no longer fluctuate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/builders/plugins/license.ts`:
- Around line 44-49: The code currently deduplicates using the bare pkgName
which collapses different versions; instead deduplicate by the package root path
or package.json identity: compute the package root (pkgDir) or resolve its real
path (e.g., realPkgDir = fs.realpathSync(pkgDir)) and use that as the key in
seen (replace seen.has(pkgName)/seen.add(pkgName) with
seen.has(realPkgDir)/seen.add(realPkgDir)); alternatively read pkgJsonPath and
dedupe by `${pkgJson.name}@${pkgJson.version}` to ensure different versions are
kept.
- Around line 11-12: The search for node_modules in license.ts uses
platform-dependent separators (path.sep); change the logic that checks module
IDs to use POSIX-style forward slashes ("/node_modules/") or normalize IDs to
.replaceAll("\\", "/") before parsing so it matches Rolldown's normalized IDs;
update any references in functions that parse module IDs (e.g., the code that
currently uses dirname/join/sep imports) to perform this normalization and then
slice/regex against "/node_modules/" so Windows paths resolve correctly.
---
Nitpick comments:
In `@test/fixture/package.json`:
- Around line 17-18: In test/fixture/package.json replace the wildcard
devDependency for "defu" ("*") with an exact semver string matching the version
recorded in your lockfile or currently installed in node_modules (so the fixture
is reproducible); update the "defu" entry under devDependencies to that exact
version and re-run the snapshot/packaging tests to verify the
THIRD-PARTY-LICENSES.md and bundled chunk lists no longer fluctuate.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 532c1f5f-7acc-43f3-a084-b78f38568f96
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamltest/__snapshots__/obuild.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (5)
package.jsonsrc/builders/plugins/license.tstest/fixture/package.jsontest/fixture/src/index.tstest/obuild.test.ts
💤 Files with no reviewable changes (1)
- package.json
…lity Rolldown normalizes module IDs to use forward slashes, but the license plugin used platform-dependent `path.sep`. This caused `node_modules` lookup to fail on Windows. Use `/` directly and normalize backslashes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/builders/plugins/license.ts (1)
179-189: Consider usingconsolafor consistent logging.The codebase uses
consolafor logging (e.g.,bundle.tsline 29). Usingconsole.loghere is inconsistent with the established pattern.Suggested fix
Add the import at the top of the file:
import { consola } from "consola";Then update the logging calls:
- console.log("Appending third-party licenses to", output); + consola.log("Appending third-party licenses to", output); ... - console.log("Writing third-party licenses to", output); + consola.log("Writing third-party licenses to", output);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/builders/plugins/license.ts` around lines 179 - 189, Replace inconsistent console.log usage in the license builder with the project's consola logger: add the import "import { consola } from 'consola';" at the top of the file and replace the two console.log calls that print "Appending third-party licenses to" and "Writing third-party licenses to" with the appropriate consola method (e.g., consola.info) so logging matches bundle.ts and the rest of the codebase.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/builders/plugins/license.ts`:
- Around line 179-189: Replace inconsistent console.log usage in the license
builder with the project's consola logger: add the import "import { consola }
from 'consola';" at the top of the file and replace the two console.log calls
that print "Appending third-party licenses to" and "Writing third-party licenses
to" with the appropriate consola method (e.g., consola.info) so logging matches
bundle.ts and the rest of the codebase.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 29d1a8fe-d2e0-4e28-89ac-2bbd5f0b7e4e
📒 Files selected for processing (1)
src/builders/plugins/license.ts
2 participants
Warning
This PR was generated via vibe coding (one-shot by Claude Code) and has not been self-reviewed yet. Feedback and review are very welcome.
Motivation
rollup-plugin-licenseis a Rollup plugin being used in obuild with type hacks (as unknown,@ts-expect-error) to work with Rolldown. It also pulls in heavy transitive dependencies likelodashandmoment— unnecessary weight for a build tool.Inspired by You-Dont-Need-Lodash-Underscore, this PR removes the dependency entirely.
What changed
rollup-plugin-licensewith a native Rolldown plugin (obuild:license) that usesthis.getModuleIds()in thegenerateBundlehook to collect bundled dependency info directly fromnode_modules/*/package.jsonand LICENSE files.as unknowncasts or@ts-expect-errorcomments. The plugin is now a first-class Rolldown plugin.rollup-plugin-licensefrompackage.json, eliminating these transitive dependencies:lodash,moment,commenting,fdir,magic-string(duplicate),package-name-regex,spdx-expression-validate,spdx-satisfies.What stayed the same
The license file generation logic (formatting, sorting, grouping of dependencies with identical license text,
THIRD-PARTY-LICENSES.mdoutput) is unchanged — it was already a custom implementation in this repo.📋 Claude Code Plan (used during implementation)
Context
rollup-plugin-licenseis a Rollup plugin being used with type hacks to work with Rolldown. It brings heavy transitive deps (lodash, moment, etc.). Only the dependency collection part uses the external library — the license file generation logic is already self-implemented. Using t_wada-style TDD, we capture the existing behavior with tests before replacing the implementation.Step 1: Add a bundled dependency to the test fixture
"devDependencies": { "defu": "*" }totest/fixture/package.jsondefuintest/fixture/src/index.tspnpm installdefuas a devDependency won't be marked external, so it gets bundled and triggers license collectionStep 2: Write characterization tests (Green with current implementation)
Add to
test/obuild.test.ts:THIRD-PARTY-LICENSES.mdexists# Licenses of Bundled Dependencies,MIT)defudependency info (package name, license type, author, full license text)THIRD-PARTY-LICENSES.mdStep 3: Run tests → Confirm Green Bar
pnpm vitest run test/obuild.test.ts— all tests pass with existing implementationStep 4: Replace the implementation
src/builders/plugins/license.ts:rollup-plugin-licenseimportgenerateBundlehook withthis.getModuleIds()for dependency collectionpackage.json+ LICENSE files directly fromnode_modulesas unknown,@ts-expect-error)rollup-plugin-licensefrompackage.jsonpnpm installStep 5: Run tests → Confirm Green Bar
Step 6: Verification
pnpm why lodashconfirms lodash is gonepnpm buildconfirms obuild builds successfullyTarget files
test/fixture/package.jsontest/fixture/src/index.tstest/obuild.test.tssrc/builders/plugins/license.tspackage.jsonSummary by CodeRabbit
Refactor
Chores
Tests