Conversation
🦋 Changeset detectedLatest commit: 2def2ee 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 |
WalkthroughThe changes update the size-limit configuration to monitor a different source file and increase the size threshold, revise the build script for Changes
Possibly related PRs
Suggested labels
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/helpers.tsOops! Something went wrong! :( ESLint: 9.27.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@1stg/eslint-config' imported from /eslint.config.js eslint.config.jsOops! Something went wrong! :( ESLint: 9.27.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@1stg/eslint-config' imported from /eslint.config.js ✨ 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 (
|
WalkthroughThis pull request updates various development dependencies and modifies configuration files to accommodate these changes. The updates include changes to the size limit configuration, build scripts, and import paths in test files. Changes
|
|
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. |
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to bc44c2b in 44 seconds. Click for details.
- Reviewed
84lines of code in3files - Skipped
1files when reviewing. - Skipped posting
4draft 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. .size-limit.json:3
- Draft comment:
Changed source file reference and size limit. Confirm that measuring the source ('src/index.ts') instead of the built file is intentional. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. package.json:52
- Draft comment:
Updated tsdown command: Added '--no-clean' and shorthand '-f'. Verify that the new flags produce the expected build output. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. package.json:67
- Draft comment:
Multiple devDependency versions have been bumped. Check for any breaking changes, especially with major updates like '@1stg/common-config' from ^13.0.1 to ^14.0.0. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
4. test/types-d.cts:3
- Draft comment:
Changed import path from 'synckit' to '..'. Ensure this local reference resolves as intended for type tests. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_mp4K8xfWpiGxtIYh
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Pull Request Overview
Update development dependencies and associated tooling configurations.
- Adjust test import to reference local package entry point
- Enhance the
tsdownbuild script with--no-cleanand shorthand flags - Bump multiple devDependency versions
- Change size-limit target to source file and raise the threshold
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/types-d.cts | Updated import synckit to load from the local module ('..') for testing |
| package.json | Modified build:tsdown script flags; bumped several devDependency versions |
| .size-limit.json | Switched measurement path to src/index.ts and increased size limit to 1.6kB |
Comments suppressed due to low confidence (2)
package.json:52
- [nitpick] Introducing
--no-cleanmight leave stale artifacts inlib/. If you intend to get a fresh build each time, consider removing--no-cleanor explicitly cleaning the output directory beforehand.
"build:tsdown": "tsdown --no-clean -d lib -f cjs src/index.ts",
.size-limit.json:3
- [nitpick] Measuring the TypeScript source size rather than the compiled JavaScript (
lib/index.js) may not reflect the actual package footprint. Consider pointing to the built output to get accurate bundle size metrics.
"path": "./src/index.ts",
commit: |
📊 Package size report -0.05%↓
Unchanged files
🤖 This report was automatically generated by pkg-size-action |
size-limit report 📦
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #246 +/- ##
=======================================
Coverage 95.82% 95.82%
=======================================
Files 4 4
Lines 335 335
Branches 154 154
=======================================
Hits 321 321
+ Misses 14 11 -3
- Partials 0 3 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
.size-limit.json(1 hunks)package.json(2 hunks)test/types-d.cts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (19)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js 22 on windows-latest
- GitHub Check: publish
- GitHub Check: Package Size Report
- GitHub Check: Lint and Test with Node.js 24 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 23 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 22 on windows-latest
- GitHub Check: Lint and Test with Node.js 23 on windows-latest
- GitHub Check: Lint and Test with Node.js 18.18 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 23 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18.18 on windows-latest
- GitHub Check: Lint and Test with Node.js 18.18 on macos-latest
- GitHub Check: Lint and Test with Node.js 18 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 on macos-latest
🔇 Additional comments (7)
package.json (7)
52-52: Update tsdown build script flags
Thebuild:tsdownscript now uses--no-cleanand the shorthand-f cjs. This will preserve existing output and emit CommonJS. Looks good, but ensure this aligns with your CI/build docs.
67-67: Bump @1stg/common-config to 14.0.0
This is a major version bump—please verify there are no breaking changes in your shared config that could impact linting or formatting.
73-73: Upgrade @swc/core to 1.11.29
A patch‐level bump; no action needed unless you hit new transform issues.
76-76: Update @types/node to 22.15.26
Type definitions have advanced; run a full type check (tsc --noEmit) to catch any new warnings or errors.
84-84: Upgrade eslint-plugin-jest to 28.12.0
Minor bump—verify your existing Jest lint rules still pass.
92-92: Upgrade size-limit-preset-node-lib to 0.4.0
Matches the size-limit change; no other concerns.
99-99: Upgrade tsdown to 0.12.4
Ensure this version supports the new--no-cleanand-f cjsflags used in your build script.
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed f6e746a in 1 minute and 40 seconds. Click for details.
- Reviewed
131lines of code in6files - Skipped
0files when reviewing. - Skipped posting
8draft 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. .github/workflows/autofix.yml:33
- Draft comment:
Added a Lint Fix step using 'yarn lint:es --fix'. Confirm that the lint script handles auto-fixing as expected. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. .size-limit.json:3
- Draft comment:
Updated size limit: now targeting './src/index.ts' with a limit of 1.6kB. Ensure this measurement is intended for the source file. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. eslint.config.js:11
- Draft comment:
Switched rule from 'unicorn/no-anonymous-default-export' to 'unicorn-x/no-anonymous-default-export'. Verify that the 'unicorn-x' plugin is correctly installed and configured. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
4. eslint.config.js:18
- Draft comment:
Added rule 'unicorn-x/no-nested-ternary': 'off'. Confirm that this rule is available and intended in your ESLint setup. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
5. package.json:51
- Draft comment:
Modified the 'build:tsdown' script to include '--no-clean' and use '-f' for format. Verify that this updated behavior fits the intended build process. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
6. package.json:64
- Draft comment:
Multiple devDependency versions have been bumped. Ensure all updated versions are compatible and well-tested. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
7. src/helpers.ts:370
- Draft comment:
Updated the ESLint disable comment from 'unicorn/prefer-string-replace-all' to 'unicorn-x/prefer-string-replace-all' to match the plugin change. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
8. test/types-d.cts:3
- Draft comment:
Changed import from 'synckit' to require the local module ('..'). Ensure this correctly targets the local build for type testing. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_pFSRIhlT8t0NP50J
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Signed-off-by: JounQin <admin@1stg.me>
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 38fc43a in 1 minute and 26 seconds. Click for details.
- Reviewed
133lines of code in6files - Skipped
0files when reviewing. - Skipped posting
9draft 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. .github/workflows/autofix.yml:33
- Draft comment:
Verify that running 'yarn build' before 'lint:es --fix' in the new Lint Fix step is intentional, as it adds extra overhead. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the author to verify their intention, which is against the rules. It does not provide a specific suggestion or point out a clear issue with the code.
2. .size-limit.json:3
- Draft comment:
Changing target from './lib/index.js' to './src/index.ts' may not reflect the production bundle size. Confirm this change is intended. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment raises a valid concern - measuring TypeScript source files instead of compiled JS could give inaccurate bundle size metrics. However, this is a speculative comment asking for confirmation ("Confirm this change is intended") rather than definitively pointing out an error. The author likely had a reason for this change. The change could be intentional as part of migrating to TypeScript, and the increased size limit suggests they're aware of the implications. While the concern is valid, the comment violates our rules by asking for confirmation rather than pointing out a definite issue. Delete this comment as it's speculative and asks for confirmation rather than identifying a clear problem that needs fixing.
3. eslint.config.js:11
- Draft comment:
Rule name changed to 'unicorn-x/no-anonymous-default-export'; ensure the new plugin is configured correctly. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the author to ensure that the new plugin is configured correctly after a rule name change. This falls under asking the author to ensure something is done, which is against the rules.
4. eslint.config.js:18
- Draft comment:
Disabling 'unicorn-x/no-nested-ternary'—double-check if this override is desired for your codebase. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to double-check if the override is desired, which violates the rule against asking for confirmation or double-checking intentions. It does not provide a specific suggestion or point out a clear issue.
5. package.json:52
- Draft comment:
The tsdown command now uses '--no-clean'. Confirm that preserving previous build artifacts is intended behavior. - 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 their intention regarding the use of--no-cleanin thetsdowncommand. This falls under the rule of not asking the author to confirm their intention or ensure behavior is intended. Therefore, this comment should be removed.
6. package.json:67
- Draft comment:
Bumped '@1stg/common-config' to ^14.0.0. Verify that any potential breaking changes in this dependency are addressed. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is about a dependency change and asks the author to verify potential breaking changes, which violates the rules against commenting on dependency changes and asking for verification.
7. src/helpers.ts:370
- Draft comment:
Updated disable comment to use 'unicorn-x/prefer-string-replace-all'. Ensure the new rule name matches your eslint configuration. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the author to ensure that the new rule name matches their eslint configuration. This falls under asking the author to ensure something, which is not allowed by the rules.
8. test/types-d.cts:3
- Draft comment:
Import changed from 'synckit' to '..'. Confirm that the test is now targeting the correct module source. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the author to confirm that the test is targeting the correct module source after an import change. This is a request for confirmation, which violates the rule against asking the author to confirm their intention or ensure behavior is intended.
9. src/helpers.ts:370
- Draft comment:
It looks like there might be a typo in the ESLint disable comment. The rule name is changed to 'unicorn-x/prefer-string-replace-all'. If this was unintentional, please change it back to 'unicorn/prefer-string-replace-all'. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% The comment suggests changing back to 'unicorn/prefer-string-replace-all', but this seems to be making assumptions. The change from 'unicorn' to 'unicorn-x' could be intentional - perhaps they're using a fork or modified version of the ESLint unicorn rules. Without more context about their ESLint setup, we can't assume this is a typo. I could be wrong about this being intentional - maybe it really is a typo. But the key point is that I don't have enough context to know for sure. Even if it is a typo, speculative comments asking "if this was unintentional" violate our rules. We should only keep comments that point out definite issues. Delete this comment. It's speculative and makes assumptions about the intended ESLint configuration without sufficient context.
Workflow ID: wflow_P1x0IoPkLDbhYthg
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
.size-limit.json (1)
3-4: Size limit threshold still too low for actual file size.The 1.6kB limit remains insufficient for the actual
src/index.tsfile size of ~4.6kB as previously identified.test/types-d.cts (1)
3-3: Import path change still has resolution and compatibility issues.As previously identified, using
require('..')fails because the package's "main" field points to a non-existent file and CommonJS require() isn't compatible with the ESM-only test environment.package.json (1)
52-52: Re-evaluate the--no-cleanflag onbuild:tsdown.
Retaining previous artifacts inlib/may lead to stale files or documentation mismatches. If performance is the goal, document the rationale or consider cleaning only changed files.
🧹 Nitpick comments (2)
.github/workflows/autofix.yml (2)
33-37: Ensure formatting consistency after lint fixes.
Sinceyarn lint:es --fixcan modify code, consider adding ayarn formatstep immediately afterward to enforce Prettier conventions beforeautofix.ciapplies changes.
33-37: Consolidate scripts for clarity.
You could introduce a new npm script inpackage.json, e.g."scripts": { "lint:fix": "yarn build && yarn lint:es --fix" }and then simplify this step to
run: yarn lint:fix. This centralizes logic and eases future maintenance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (7)
.changeset/soft-chefs-hang.md(1 hunks).github/workflows/autofix.yml(1 hunks).size-limit.json(1 hunks)eslint.config.js(1 hunks)package.json(2 hunks)src/helpers.ts(1 hunks)test/types-d.cts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
test/types-d.cts (1)
Learnt from: JounQin
PR: un-ts/synckit#222
File: src/helpers.ts:13-13
Timestamp: 2025-04-02T06:32:20.004Z
Learning: Tests in the synckit project run in an ESM-only environment, so CommonJS patterns like require() should not be used in test files. Always use ESM import syntax and proper mocking approaches compatible with ESM.
🔇 Additional comments (6)
eslint.config.js (2)
11-11: ESLint rule prefix update looks correct.The change from
unicorntounicorn-xprefix aligns with the dependency update mentioned in the PR objectives.
18-18: Good addition of nested ternary rule.Adding the
unicorn-x/no-nested-ternaryrule with the updated prefix maintains consistency with the other disabled rules in this configuration..changeset/soft-chefs-hang.md (1)
1-5: Changeset format and content looks correct.The changeset properly documents the patch update for republishing as the npm latest tag version.
src/helpers.ts (1)
369-371: ESLint directive updated to match new plugin prefix.
The// eslint-disable-next-line unicorn-x/prefer-string-replace-allcomment now aligns with the updatedunicorn-xrules configured ineslint.config.js. No further change needed here..github/workflows/autofix.yml (1)
33-37: Great addition of a lint-fix phase.
Runningyarn buildbeforeyarn lint:es --fixhelps catch and fix lint issues in the latest build output.package.json (1)
67-76: Verify compatibility of updated devDependencies.
Multiple devDependencies were bumped:
@1stg/common-config→ ^14.0.0@swc/core→ ^1.11.29@types/node→ ^22.15.26eslint-plugin-jest→ ^28.12.0size-limit-preset-node-lib→ ^0.4.0tsdown→ ^0.12.4Please confirm these version upgrades do not introduce breaking changes. Run the full test suite and review each package’s changelog for required migration steps.
Also applies to: 84-92, 99-99
Important
Update development dependencies, add lint fix step, and adjust configurations in various files.
package.jsonto newer versions.Lint Fixstep in.github/workflows/autofix.ymlto runyarn buildandyarn lint:es --fix../src/index.tsand limit to1.6kBin.size-limit.json.eslint.config.jsto includeunicorn-x/no-anonymous-default-exportandunicorn-x/no-nested-ternary.synckitmodule intest/types-d.cts.This description was created by
for 38fc43a. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
Chores
Tests