-
-
Notifications
You must be signed in to change notification settings - Fork 4
chore: migrate @pkgr/rollup
to tsdown
#124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe changes update the Renovate configuration to reference a schema and ignore a specific dependency, remove a TypeScript declaration file, adjust type declaration paths and scripts in the package configuration, and refactor a variable assignment in the source code for clarity. Several development dependencies are also upgraded. Changes
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/index.tsOops! Something went wrong! :( ESLint: 9.27.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@1stg/eslint-config' imported from /eslint.config.js 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms (11)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR migrates the build system from using @pkgr/rollup to tsdown and removes the buggy module-sync export while updating related configuration and dependency versions.
- Replaces inline use of import.meta.url with a dedicated constant for clarity in src/index.ts
- Updates package.json to change module/type paths, remove module-sync exports, and update dependencies
- Removes the index.d.cts file and revises .renovaterc configuration
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
File | Description |
---|---|
src/index.ts | Uses a constant for import.meta.url for clarity |
package.json | Adjusts paths, removes module-sync, updates deps |
index.d.cts | Removed as part of the migration |
.renovaterc | Updated configuration with schema and ignores |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #124 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 4 4
Lines 76 77 +1
Branches 29 29
=========================================
+ Hits 76 77 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
size-limit report 📦
|
commit: |
📊 Package size report 0.3%↑
Unchanged files
🤖 This report was automatically generated by pkg-size-action |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed everything up to 2d0ac3f in 1 minute and 4 seconds. Click for details.
- Reviewed
136
lines of code in4
files - Skipped
1
files when reviewing. - Skipped posting
6
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .renovaterc:2
- Draft comment:
Added $schema for Renovate config validation. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
2. package.json:15
- Draft comment:
Updated 'types' path to './lib/index.d.cts'; ensure consumers adjust if needed. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
3. package.json:24
- Draft comment:
Removed 'module-sync' export block; confirm its removal is intended. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
4. package.json:47
- Draft comment:
Replaced the 'build:r' script with 'build:tsdown'; verify build output remains as expected. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
5. package.json:87
- Draft comment:
Migrated from @pkgr/rollup to tsdown in devDependencies; ensure behavior aligns with expectations. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
6. src/index.ts:11
- Draft comment:
Refactored _dirname calculation using the importMetaUrl variable for clarity. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
Workflow ID: wflow_qqBkEp7yu3pTyU3x
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
🦋 Changeset detectedLatest commit: 211a416 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 |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed 211a416 in 1 minute and 40 seconds. Click for details.
- Reviewed
145
lines of code in5
files - Skipped
0
files when reviewing. - Skipped posting
6
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .changeset/ninety-lizards-cross.md:5
- Draft comment:
Changelog entry is clear and documents the removal of the buggy 'module-sync' export. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
2. .renovaterc:2
- Draft comment:
Including the $schema key improves configuration validation for Renovate. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
3. package.json:15
- Draft comment:
Updated the 'types' path to './lib/index.d.cts'. Please verify that the generated declaration files are indeed output to the lib directory. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to verify the output path of generated declaration files, which falls under asking the author to double-check or ensure something. This violates the rule against asking the PR author to confirm or ensure behavior.
4. package.json:27
- Draft comment:
The removal of the 'module-sync' export block is noted. Ensure that no external consumers depend on this export. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to ensure that no external consumers depend on the removed export. This falls under asking the author to double-check things, which is against the rules.
5. package.json:47
- Draft comment:
New 'build:tsdown' script added. Confirm that the external path '../vendors/wasm_exec.cjs' is correctly resolved relative to the output directory. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to confirm the resolution of an external path, which is not allowed according to the rules. It doesn't provide a specific suggestion or point out a clear issue with the code.
6. src/index.ts:11
- Draft comment:
Refactored _dirname assignment by storing import.meta.url in a variable. This improves clarity—consider if the __dirname fallback is necessary in a pure ESM context. - Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_0XZa1I0gfbRmorRl
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Also removed buggy
module-sync
: nodejs/node#57869Important
Migrated from
@pkgr/rollup
totsdown
, removedmodule-sync
export, updated dependencies, and modified build scripts.@pkgr/rollup
totsdown
.build:tsdown
script inpackage.json
for generating CommonJS type declarations.module-sync
export frompackage.json
due to a bug.devDependencies
inpackage.json
, including@1stg/common-config
,@changesets/cli
, and others..renovaterc
to ignoremvdan-sh
._dirname
initialization insrc/index.ts
for clarity.index.d.cts
file and its references inpackage.json
.This description was created by
for 211a416. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit