chore: bump all (dev)Dependencies and fix related usage issues #206
Conversation
🦋 Changeset detectedLatest commit: ca918ce 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 project configuration, dependencies, and code structure. Notably, the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GitModule as git.ts
participant HgModule as hg.ts
participant FindUp as @pkgr/core
User->>GitModule: detect(directory)
GitModule->>FindUp: findUp(resolved directory, '.git', true)
FindUp-->>GitModule: path to .git or null
GitModule-->>User: parent directory or null
User->>HgModule: detect(directory)
HgModule->>FindUp: findUp(resolved directory, '.hg', true)
FindUp-->>HgModule: path to .hg or null
HgModule->>HgModule: check if directory
HgModule-->>User: parent directory or undefined
Assessment against linked issues
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
.simple-git-hooks.mjsOops! Something went wrong! :( ESLint: 9.28.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@1stg/eslint-config' imported from /eslint.config.mjs .nano-staged.mjsOops! Something went wrong! :( ESLint: 9.28.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@1stg/eslint-config' imported from /eslint.config.mjs eslint.config.mjsOops! Something went wrong! :( ESLint: 9.28.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@1stg/eslint-config' imported from /eslint.config.mjs
✨ 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 (
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to 00cd0c6 in 1 minute and 43 seconds. Click for details.
- Reviewed
539lines of code in22files - Skipped
1files 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. .prettierignore:1
- Draft comment:
Ensure the removal of 'coverage' and 'lib' ignore patterns is intentional, as it changes which files get formatted. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. package.json:15
- Draft comment:
The engines.node version is still set to '>=14' while .nvmrc is updated to '20'. Consider aligning these for consistency. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. src/scms/git.ts:9
- Draft comment:
Usage of findUp from '@pkgr/core' simplifies detection; consider adding an explicit type annotation for clarity of its return type. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
4. src/processFiles.ts:31
- Draft comment:
Synchronous file operations (readFileSync/writeFileSync) may block the event loop. Consider async I/O if performance becomes an issue. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
5. src/createIgnorer.ts:1
- Draft comment:
There appears to be a possible typo in the eslint-disable directive. The rule is currently specified as "unicorn-x/filename-case" - did you mean "unicorn/filename-case"? - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The change from 'unicorn/' to 'unicorn-x/' looks intentional since it's the only change in that line. Without access to the project's eslint configuration or knowledge of which eslint plugins are used, I can't be certain whether this is actually a typo or an intentional change to a different plugin. The author might have deliberately changed it to use a forked or different version of the rule. I might be too cautious - a hyphen in an eslint plugin name is unusual and could genuinely be a typo that would cause the directive to fail. Even if it looks unusual, without access to the project's eslint configuration, I can't be certain this is incorrect. The change appears deliberate, and if it were wrong, eslint would catch it during build. The comment should be deleted because eslint configuration issues would be caught by the build process, and we can't be certain this is actually incorrect without more context.
6. src/createMatcher.ts:1
- Draft comment:
The ESLint directive was changed fromunicorn/filename-casetounicorn-x/filename-case. This change looks like it might be a typographical error. Please confirm that the new rule name (unicorn-x/filename-case) is intended. - Reason this comment was not posted:
Comment looked like it was already resolved.
7. src/processFiles.ts:1
- Draft comment:
Typo alert: The change from 'unicorn/filename-case' to 'unicorn-x/filename-case' in the eslint disable comment on line 1 appears to be a typographical error. Please verify if the inserted-xis intentional. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While this is about a changed line, eslint rules and configurations are essentially dependency/configuration issues. Without access to the project's eslint configuration or knowledge of which eslint plugins are installed, I can't be certain whether 'unicorn-x' is incorrect or if it's a valid plugin name that was intentionally changed. The change could be intentional if they're switching to a forked version of the eslint-plugin-unicorn package. The comment might be highlighting a real issue. Even if it's a real issue, this falls under the rule about not commenting on dependency changes or library versions we don't recognize. The build system would catch an invalid eslint rule. Delete the comment as it relates to eslint configuration/dependencies, and any invalid eslint rule would be caught by the build system anyway.
8. test/isSupportedExtension.spec.ts:1
- Draft comment:
The rule name appears to have changed from 'unicorn/filename-case' to 'unicorn-x/filename-case'. This seems like a typo. Please double-check if the intended rule name is 'unicorn/filename-case' and correct it if necessary. - 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 change from 'unicorn' to 'unicorn-x' seems deliberate rather than accidental. Without access to the project's ESLint configuration or knowledge about which ESLint plugins are being used, I cannot definitively say this is wrong. The change could be due to a plugin rename or migration. The comment is speculative by asking to "double-check" rather than pointing out a definitive issue. I might be wrong about the intentionality - it could genuinely be a typo. Also, the suggestion to revert might be correct even if I can't be certain. While it could be a typo, without clear evidence that this is incorrect (like access to the ESLint config), we should assume the author made this change intentionally. The comment is also phrased as a request to verify rather than pointing out a definitive issue. Delete this comment as it violates the rules against speculative comments and asking authors to double-check things. Without access to ESLint configuration, we can't be certain this is incorrect.
9. tsconfig.lib.json:2
- Draft comment:
Typo: In the comment, 'do not includes paths' appears to have an incorrect verb form. It might be better written as 'do not include paths' or 'does not include paths'. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_rsN5ocF4dLoYws7J
You can customize 
There was a problem hiding this comment.
Pull Request Overview
This PR updates dependencies across the project and adapts code to breaking API changes in those dependencies.
- Replace
find-upwith@pkgr/core’sfindUpin SCM modules - Rename default arrow-exported functions to named functions for clearer stack traces
- Bump dev and peer dependencies, adjust scripts, package
"exports", and config files
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/scm-git.spec.ts | Removed duplicated eslint-disable comments for sonarjs rules |
| test/isSupportedExtension.spec.ts | Updated eslint-disable rule for filename-case plugin |
| src/scms/index.ts | Changed anonymous default export to detectScm named function |
| src/scms/hg.ts | Switched to @pkgr/core’s findUp and simplified detection |
| src/scms/git.ts | Simplified .git detection using findUp |
| src/processFiles.ts | Renamed default export to processFiles named function |
| src/isSupportedExtension.ts | Refactored default export into isSupportedExtension constant |
| src/index.ts | Renamed default export to prettyQuick named function |
| src/createMatcher.ts | Named default export createMatcher |
| src/createIgnorer.ts | Named default export createIgnorer |
| package.json | Bumped devDependencies, added "type", restructured "exports" |
| eslint.config.mjs | New ESLint config with overrides |
| .yarnrc.yml | Normalized quoting in plugin specs |
| .simple-git-hooks.mjs/.js | Switched hook file to .mjs |
| .prettierignore | Simplified ignore patterns |
| .nvmrc | Updated Node version from 18 to 20 |
| .nano-staged.mjs | Added nano-staged entry |
| .lintstagedrc.js | Removed legacy lint-staged config |
| .github/FUNDING.yml | Updated funding and contributor links |
Comments suppressed due to low confidence (4)
test/scm-git.spec.ts:59
- [nitpick] Removing the disable for
no-duplicate-stringcan lead to lint failures on repeated literals like 'merge-base'. Either refactor to reuse constants or re-add the disable for clarity.
// eslint-disable-next-line sonarjs/no-duplicate-string
src/isSupportedExtension.ts:11
- The refactored default export into a named constant isn't covered by existing tests. Consider adding unit tests to ensure the exported function behaves correctly when
resolveConfigis bothtrueandfalse.
const isSupportedExtension =
package.json:4
- [nitpick] Declaring
"type": "commonjs"while shipping.mjsmodules may confuse ESM consumers. Verify your module type and file extensions align, or switch to"type": "module"if you intend pure ESM.
"type": "commonjs",
src/scms/hg.ts:10
- This code uses
fs.statSyncafterfindUp, which blocks the event loop. Consider using the asynchronousfs.statto avoid potential startup delays, especially in large repos.
const found = findUp(path.resolve(directory), '.hg', true)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
test/isSupportedExtension.spec.ts (1)
6-6:⚠️ Potential issueResolve named‐export vs default‐import mismatch
According to the PR summary,src/isSupportedExtension.tsnow exports a named constantisSupportedExtensioninstead of a default. The test currently does:import isSupportedExtension from 'pretty-quick/isSupportedExtension'This will break. It should be:
-import isSupportedExtension from 'pretty-quick/isSupportedExtension' +import { isSupportedExtension } from 'pretty-quick/isSupportedExtension'
🧹 Nitpick comments (2)
tsconfig.lib.json (1)
2-2: Nit: refine inline comment grammar
The comment// used by all files but do not includes pathscould be clearer as:// used by all files but does not include paths🧰 Tools
🪛 Biome (1.9.4)
[error] 2-2: Expected a property but instead found '// used by all files but do not includes paths'.
Expected a property here.
(parse)
src/scms/index.ts (1)
6-6: Name the exported function for improved stack traces.Converting the anonymous default export to a named
detectScmfunction enhances readability and debugging. Consider adding an explicit return type annotation to the signature for stronger type safety, e.g.:export default function detectScm( directory: string ): { rootDirectory: string; name: string; detect: (dir: string) => string | undefined } | undefined { // ... }
🛑 Comments failed to post (1)
src/scms/hg.ts (1)
10-14: 🛠️ Refactor suggestion
Consider error handling and consistency with git.ts implementation.
The implementation adds a directory check that's not present in git.ts. Consider:
- Error handling:
fs.statSync(found)can throw if the file doesn't exist or has permission issues- Consistency: git.ts doesn't have this directory check - should both SCM implementations behave the same way?
Consider adding error handling:
export const detect = (directory: string) => { const found = findUp(path.resolve(directory), '.hg', true) - if (found && fs.statSync(found).isDirectory()) { + if (found) { + try { + if (fs.statSync(found).isDirectory()) { + return path.dirname(found) + } + } catch (error) { + // Handle permission errors or race conditions + return undefined + } + } - return path.dirname(found) - } }Also verify if git.ts should have similar directory validation for consistency.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export const detect = (directory: string) => { const found = findUp(path.resolve(directory), '.hg', true) if (found) { try { if (fs.statSync(found).isDirectory()) { return path.dirname(found) } } catch (error) { // Handle permission errors or race conditions return undefined } } }🤖 Prompt for AI Agents
In src/scms/hg.ts around lines 10 to 14, the code uses fs.statSync(found) without error handling, which can throw exceptions if the path is inaccessible. To fix this, wrap the fs.statSync call in a try-catch block to handle potential errors gracefully. Additionally, review the git.ts implementation to check if it should also include a directory existence check for consistency between SCM implementations, and update git.ts accordingly if needed.
|
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. |
size-limit report 📦
|
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 51a5c94 in 1 minute and 35 seconds. Click for details.
- Reviewed
541lines of code in22files - 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. package.json:14
- Draft comment:
The engines.node is still ">=14" while .nvmrc is "20". Consider updating engines to ">=20" for consistency. - Reason this comment was not posted:
Comment was on unchanged code.
2. test/scm-git.spec.ts:82
- Draft comment:
Test 'with --staged does NOT call git merge-base' does not pass the staged flag. It should call prettyQuick with { staged: true } to align with intended logic. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. .prettierignore:1
- Draft comment:
Review if the removal of 'coverage' and 'lib' from .prettierignore is intentional; formatting generated files may be undesirable. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment is speculative - it asks to "review if" something is intentional rather than pointing out a definite issue. It's asking the author to double-check their intention, which violates our rules. We don't have strong evidence that keeping these directories in .prettierignore is necessary. Maybe the reviewer has experience with these being generated files that shouldn't be formatted, and this could cause real issues. Without concrete evidence that these are actually generated files in this codebase, or that formatting them would cause problems, we shouldn't keep speculative comments asking for verification. Delete this comment as it violates our rules against asking authors to verify intentions and making speculative comments without strong evidence.
4. src/index.ts:30
- Draft comment:
Typographical issue: There's an unexpected closing parenthesis on line 30 () {) in the function declaration. It appears the intended change was to remove the trailing)and have just an opening brace, e.g.,{. Please fix this. - Reason this comment was not posted:
Comment looked like it was already resolved.
5. src/isSupportedExtension.ts:1
- Draft comment:
There seems to be a typographical error on line 1 in the updated ESLint disable comment. The rule is now referring tounicorn-x/filename-case. Is this intentional? It appears that it might have been meant to remainunicorn/filename-case. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While the comment identifies a real change, it's likely this change was intentional as part of updating or changing ESLint configurations. ESLint rules and their naming are part of the build/lint configuration, which falls under our "do not comment on build-related changes" rule. Additionally, if this were incorrect, it would be caught by ESLint itself during the build. The change could be unintentional and cause linting to silently fail if the rule name is wrong. Maybe this deserves attention? No - if the rule name is wrong, ESLint will throw an error during the build process. This is exactly the kind of issue that would be caught automatically. Delete the comment. Changes to ESLint configuration and rule names are build-related changes that would be caught by the build process if incorrect.
6. src/processFiles.ts:1
- Draft comment:
Typographical error: The rule name was changed fromunicorn/filename-casetounicorn-x/filename-case. Confirm if the-xsuffix is intentional, or if it’s a typo. - 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% This is a dependency/configuration related change - it's about an ESLint rule name. According to our rules, we should not comment on dependency changes or anything related to dependencies. Even if it was a typo, it would be caught by ESLint itself when running. The author likely made this change intentionally as part of updating their ESLint configuration. Maybe this really is a typo that could cause issues with linting. Maybe we should verify it. No - per our rules, we explicitly don't comment on dependency or configuration changes. If it's wrong, ESLint itself will catch it when it runs. Delete this comment as it relates to dependency/configuration changes which we're instructed to ignore. If it's a real issue, ESLint will catch it.
7. src/processFiles.ts:20
- Draft comment:
Typo: It looks like there's a stray closing parenthesis on line 20. It should be removed so that the opening brace directly follows the parameter default, i.e., changing from): {to just{. - Reason this comment was not posted:
Comment looked like it was already resolved.
8. test/isSupportedExtension.spec.ts:1
- Draft comment:
Typographical note: The ESLint disable directive now readsunicorn-x/filename-case(with an extra-x). Please confirm if this is an intentional change. If not, it should likely be reverted back tounicorn/filename-case. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment points out a potential typo in an ESLint directive. However, we don't have enough context to know if 'unicorn-x' is a valid ESLint plugin name - it could be a fork or custom version. Also, linting configuration is often project-specific and we shouldn't make assumptions about what plugins should be used. I might be too hesitant - if 'unicorn-x' is truly a typo, this could cause the linting directive to fail silently and the file would be incorrectly linted. Even if it is a typo, linting issues would be caught by the build system, and our rules specifically say not to comment on things that would be caught by the build. The comment should be deleted as it relates to a linting configuration that would be caught by the build if incorrect.
9. tsconfig.lib.json:2
- Draft comment:
Typographical error: In the comment, 'used by all files but do not includes paths' should likely be 'used by all files but do not include paths'. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_e7IIEKLWncNTo7cQ
You can customize
commit: |
📊 Package size report -8.59%↓
Unchanged files
🤖 This report was automatically generated by pkg-size-action |
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 6c554d8 in 40 seconds. Click for details.
- Reviewed
257lines of code in7files - Skipped
0files when reviewing. - Skipped posting
2draft 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/ci.yml:15
- Draft comment:
Consider updating the Node.js matrix to only include versions >=20 to align with the new engines requirement. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. .github/workflows/release.yml:41
- Draft comment:
Pin the 'changesets/action' to an explicit commit for consistency and enhanced security instead of using 'v1'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_YHtjiiHVIxqzy7rt
You can customize
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #206 +/- ##
==========================================
+ Coverage 85.45% 91.61% +6.16%
==========================================
Files 8 9 +1
Lines 165 167 +2
Branches 49 36 -13
==========================================
+ Hits 141 153 +12
+ Misses 21 12 -9
+ Partials 3 2 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 6651a5d in 46 seconds. Click for details.
- Reviewed
266lines of code in7files - Skipped
1files when reviewing. - Skipped posting
3draft 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:19
- Draft comment:
Good job pinning the checkout action's commit hash – this enhances build reproducibility and security. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. .github/workflows/ci.yml:47
- Draft comment:
The lint step is skipped for Node 14 and 16; consider adding a comment or documentation to clarify why these versions are excluded. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. .github/workflows/codeql.yml:1
- Draft comment:
The entire CodeQL workflow was removed. Confirm that this removal is intentional, as it might affect code scanning or security analysis processes. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_V2rHJLVOmaQom2A2
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 2a17273 in 53 seconds. Click for details.
- Reviewed
270lines of code in7files - Skipped
1files when reviewing. - Skipped posting
3draft 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/release.yml:41
- Draft comment:
Pin the changesets/action reference to a specific commit hash (instead of @v1) for improved security and consistency with the other actions. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. .github/workflows/ci.yml:48
- Draft comment:
Verify that skipping the lint step on Node.js 14 and 16 (if condition) is intentional. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. .github/workflows/ci.yml:27
- Draft comment:
Consider extracting common steps (checkout, setup-node, install dependencies) into a reusable workflow to reduce duplication across workflows. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
Workflow ID: wflow_hT3KS1vt7pMKek6p
You can customize
There was a problem hiding this comment.
Pull Request Overview
This PR updates dependencies and configuration files, refactors function exports, and enhances GitHub Actions workflows while bumping various (dev)dependencies. Key changes include:
- Refactoring anonymous functions to named functions in src/createMatcher.ts and src/createIgnorer.ts.
- Upgrading dependency versions and updating configuration files (e.g. package.json, ESLint, Yarn).
- Adding and updating multiple GitHub Actions workflows and patches (including a patch for ts-jest).
Reviewed Changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/createMatcher.ts and src/createIgnorer.ts | Refactored exported functions to named declarations for clarity |
| patches/ts-jest+29.3.4.patch | Updated patch logic for transpile-worker in ts-jest |
| package.json | Bumped dependency versions, updated module exports, and Node version |
| eslint.config.mjs, .yarnrc.yml, etc. | Added/modified configuration files for linting and package management |
| GitHub workflows (ci.yml, release.yml, etc.) | Enhanced GitHub Actions workflows with concurrency and version updates |
Comments suppressed due to low confidence (2)
package.json:15
- The engines field still specifies '>=14' for Node, which is inconsistent with the updated Node version (20) in .nvmrc. Update this to reflect the new minimum Node version.
"node": ">=14"
patches/ts-jest+29.3.4.patch:9
- The change from using the nullish coalescing assignment operator to a ternary expression may alter the intended behavior. Please verify that this logic change preserves the original functionality.
- barebonesLibSourceFile ??= typescript_1.default.createSourceFile(barebonesLibName, barebonesLibContent, {
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 0f4027f in 1 minute and 31 seconds. Click for details.
- Reviewed
292lines of code in7files - Skipped
1files when reviewing. - Skipped posting
7draft 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:1
- Draft comment:
Workflow name 'autofix.ci' and its configuration are correctly set for the autofix-ci action. Pinned commit hashes and settings look good. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. .github/workflows/ci.yml:50
- Draft comment:
The conditional steps (skipping build on Node 14 and lint on Node 14/16) appear intentional. Confirm that these omissions align with your testing strategy. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. .github/workflows/pkg-pr-new.yml:34
- Draft comment:
The publish workflow is clearly defined with pinned action versions and proper steps. No issues noted. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. .github/workflows/pkg-size.yml:22
- Draft comment:
The Package Size Report workflow is configured correctly with continue-on-error to avoid blocking PRs on size-report failures. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
5. .github/workflows/release.yml:40
- Draft comment:
The release workflow uses Changesets with proper release parameters and environment variables (including NPM_CONFIG_PROVENANCE) to ensure secure releases. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
6. .github/workflows/size-limit.yml:30
- Draft comment:
The Size Limit workflow is well set up using the updated size-limit-action and a custom script parameter, ensuring proper size reporting. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
7. .github/workflows/size-limit.yml:30
- Draft comment:
There appears to be an unusual version comment on the size-limit-action usage. The version is shown as "v1.8.0.8.0" – please verify if this is intentional or if it might be a typographical error (perhaps it should be "v1.8.0"). - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment is about a real change in the diff. However, it's asking for verification rather than pointing out a clear issue. The use of commit hashes for GitHub Actions is actually a security best practice, and the version comment after the hash is just for human readability. Even if the version comment is wrong, it doesn't affect functionality since the commit hash is what matters. The version number could genuinely be incorrect and misleading to future maintainers. Documentation accuracy is important. While documentation accuracy matters, this comment violates our rules by asking for verification rather than pointing out a clear issue. The commit hash is what determines behavior, not the version comment. Delete the comment. It asks for verification rather than pointing out a clear issue, and the version comment is not functionally important since the commit hash is what matters.
Workflow ID: wflow_pmR9Uw1GD3MzH6h3
You can customize
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/ci.yml (2)
31-31: Pin action to commit SHA for reproducibility
Lockingactions/checkoutto a specific commit ensures stability, but consider using the equivalent semantic version tag (e.g.,@v4.2.2) for improved readability and easier updates.
34-34: Pin Node setup action for deterministic builds
Pinningactions/setup-nodeto a SHA is good for reproducibility. You might opt to use the official@v4.4.0tag for clarity in the workflow.tsconfig.lib.json (1)
2-2: Nit: refine comment grammar.
The inline comment at line 2 has a small grammatical issue. Consider updating it for clarity, for example:// Used by all library files; path mappings are configured in tsconfig.paths.json.🧰 Tools
🪛 Biome (1.9.4)
[error] 2-2: Expected a property but instead found '// used by all files but do not includes paths'.
Expected a property here.
(parse)
📜 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 (31)
.changeset/chatty-pumas-applaud.md(1 hunks).github/FUNDING.yml(1 hunks).github/workflows/autofix.yml(1 hunks).github/workflows/ci.yml(3 hunks).github/workflows/codeql.yml(0 hunks).github/workflows/pkg-pr-new.yml(1 hunks).github/workflows/pkg-size.yml(1 hunks).github/workflows/release.yml(2 hunks).github/workflows/size-limit.yml(1 hunks).lintstagedrc.js(0 hunks).nano-staged.mjs(1 hunks).nvmrc(1 hunks).prettierignore(1 hunks).simple-git-hooks.js(0 hunks).simple-git-hooks.mjs(1 hunks).yarnrc.yml(1 hunks)eslint.config.mjs(1 hunks)package.json(3 hunks)patches/ts-jest+29.3.4.patch(1 hunks)src/createIgnorer.ts(1 hunks)src/createMatcher.ts(1 hunks)src/index.ts(2 hunks)src/isSupportedExtension.ts(2 hunks)src/processFiles.ts(3 hunks)src/scms/git.ts(1 hunks)src/scms/hg.ts(1 hunks)src/scms/index.ts(1 hunks)test/isSupportedExtension.spec.ts(1 hunks)test/scm-git.spec.ts(0 hunks)test/scm-hg.spec.ts(0 hunks)tsconfig.lib.json(1 hunks)
💤 Files with no reviewable changes (5)
- test/scm-hg.spec.ts
- .github/workflows/codeql.yml
- .simple-git-hooks.js
- .lintstagedrc.js
- test/scm-git.spec.ts
✅ Files skipped from review due to trivial changes (7)
- .changeset/chatty-pumas-applaud.md
- .github/workflows/release.yml
- .yarnrc.yml
- .github/workflows/autofix.yml
- .github/workflows/size-limit.yml
- .github/workflows/pkg-size.yml
- .github/workflows/pkg-pr-new.yml
🚧 Files skipped from review as they are similar to previous changes (16)
- .github/FUNDING.yml
- .nvmrc
- .simple-git-hooks.mjs
- .nano-staged.mjs
- test/isSupportedExtension.spec.ts
- .prettierignore
- eslint.config.mjs
- src/scms/index.ts
- src/createIgnorer.ts
- src/index.ts
- src/processFiles.ts
- src/scms/hg.ts
- src/createMatcher.ts
- src/scms/git.ts
- src/isSupportedExtension.ts
- package.json
🧰 Additional context used
🪛 Biome (1.9.4)
tsconfig.lib.json
[error] 3-3: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 3-3: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 3-3: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 3-3: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 4-4: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 4-4: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 5-5: Expected a property but instead found '// https://github.com/microsoft/TypeScript/wiki/Node-Target-Mapping#node-14'.
Expected a property here.
(parse)
[error] 4-4: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 5-5: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 6-6: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 6-6: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 6-6: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🔇 Additional comments (8)
.github/workflows/ci.yml (5)
11-12: Review scoped permissions
You've restrictedpermissionsto onlycontents: read, which is a strong security baseline. However, since you're using the Codecov action (which may post GitHub Checks), please verify whether additional permissions such aschecks: writeare required to ensure full functionality.
27-27: Batch matrix runs for full result
Settingfail-fast: falseensures that all matrix permutations run to completion, giving visibility into all failures. This is helpful for debugging multi-version support.
42-44: Skip build on Node 14 for compatibility
Conditionalif: matrix.node != 14skips the build step on Node.js 14, which may be necessary if certain build tools aren't supported. Please verify that downstream steps (especially tests) do not rely on build artifacts when running on Node 14.
46-47: Run tests unconditionally across versions
Moving theTeststep outside anyifensures all matrix nodes execute tests, improving consistency of coverage and feedback across environments.
57-59: Secure Codecov upload with explicit token
Pinningcodecov-actionto a commit and explicitly supplyingtoken: ${{ secrets.CODECOV_TOKEN }}follows best practices for both security and reproducibility.patches/ts-jest+29.3.4.patch (1)
9-13:❓ Verification inconclusive
Verify the necessity of this compatibility workaround.
The patch replaces the nullish coalescing assignment operator (
??=) with explicit conditional logic. While functionally equivalent, this suggests a compatibility issue with your build environment or target JavaScript version.The replacement logic is correct:
== nullproperly checks for bothnullandundefined- The ternary operator maintains the same assignment behavior
- Parentheses and leading semicolon prevent parsing issues
However, consider these points:
- Root cause: Determine if this is due to Node.js version, TypeScript target, or build tool limitations
- Temporary measure: This patch should ideally be temporary until ts-jest or your toolchain supports ES2021 features
- Maintenance burden: Dependency patches require updates when the package version changes
🌐 Web query:
What is the minimum Node.js version that supports the nullish coalescing assignment operator (??=)?💡 Result:
The nullish coalescing assignment operator (
??=) was introduced in ECMAScript 2021 (ES12). Node.js added support for this operator in version 15.0.0. (www-igm.univ-mlv.fr) Therefore, to use the??=operator in Node.js, you need to be running version 15.0.0 or later.Citations:
Verify Node.js/ESTarget Compatibility for
??=OperatorThe nullish coalescing assignment operator (
??=) was added in ES2021 and is supported natively in Node.js 15.0.0 and later. If your project’s runtime or build targets Node.js < 15, this explicit== nullcheck is required; otherwise you could restore the more concise??=syntax.• Confirm your minimum Node.js version (must be ≥ 15.0.0 for
??=)
• Check your TypeScripttarget/libsettings—TS can down-level compile??=if configured
• If you upgrade your toolchain to support ES2021, consider reverting this patch to reduce maintenance overhead
• Otherwise, document this workaround clearly in your dependency-patch READMEtsconfig.lib.json (2)
3-3: Validate updated base config.
You’ve switched the"extends"from the Node 16–specific config to the generic@1stg/tsconfig/node. Please ensure that this package version aligns with your Node.js 20 environment and is installed in yourdevDependencies.🧰 Tools
🪛 Biome (1.9.4)
[error] 3-3: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 3-3: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 3-3: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 3-3: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
4-7: Verify compilation target.
You’ve set"target": "ES2020", which matches Node 14’s feature set. Since you’ve bumped.nvmrc(and presumably runtime) to Node 20, confirm whether you still want to target ES2020 for broader compatibility or bump to a newer level (ES2021/ES2022/ESNext) to leverage more modern language features.Can you confirm your intended minimum ECMAScript target?
🧰 Tools
🪛 Biome (1.9.4)
[error] 4-4: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 4-4: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 5-5: Expected a property but instead found '// https://github.com/microsoft/TypeScript/wiki/Node-Target-Mapping#node-14'.
Expected a property here.
(parse)
[error] 4-4: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 5-5: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 6-6: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 6-6: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 6-6: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
close #205
Important
Update dependencies, refactor code, and enhance workflows with new configurations and GitHub Actions.
.nvmrcandpackage.json.package.json, removing unused packages and updating versions.eslint.config.mjsand.nano-staged.mjsfor ESLint and nano-staged configuration..yarnrc.ymlfor plugin specifications..prettierignoreto ignore.yarn.src/to use named declarations.package.json.autofix.yml,pkg-pr-new.yml,pkg-size.yml.ci.yml,release.yml,size-limit.yml..github/FUNDING.yml..lintstagedrc.jsand.simple-git-hooks.js.This description was created by
for 0f4027f. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores