-
-
Notifications
You must be signed in to change notification settings - Fork 176
chore: bump all compatible (dev) deps #1171
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
🦋 Changeset detectedLatest commit: 4259112 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.
Copilot reviewed 16 out of 25 changed files in this pull request and generated 1 comment.
Files not reviewed (9)
- .editorconfig: Language not supported
- .env.yarn: Language not supported
- .gitattributes: Language not supported
- .github/dependabot.yml: Language not supported
- .husky/pre-commit: Language not supported
- .nvmrc: Language not supported
- .prettierrc.json: Language not supported
- .simple-git-hooks.json: Language not supported
- package.json: Language not supported
Comments suppressed due to low confidence (2)
package-scripts.js:55
- Ensure that the 'series' function is properly imported or defined; otherwise, this may lead to runtime errors.
script: series('clean-pkg-json', 'changeset publish'),
.eslintrc.js:8
- Confirm that using 'latest' for the ecmaVersion is supported by your current ESLint setup, as it traditionally expects a numeric value.
ecmaVersion: 'latest'
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.
❌ Changes requested. Reviewed everything up to 40ddcb1 in 2 minutes and 35 seconds
More details
- Looked at
689lines of code in22files - Skipped
3files when reviewing. - Skipped posting
16drafted comments based on config settings.
1. .husky/pre-commit:1
- Draft comment:
Removal of the pre-commit hook file; ensure replacement (e.g. simple-git-hooks) covers all needed validations. - 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. .nvmrc:1
- Draft comment:
Node version updated from 16.20.2 to v20.19.0; verify that all dependencies and CI jobs are compatible with Node 20. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to verify compatibility with the new Node version, which is against the rules. It does not provide a specific suggestion or point out a specific issue with the code.
3. package.json:84
- Draft comment:
TypeScript version updated to '~5.3.3'; check that this change doesn’t break any existing type definitions. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to check for potential issues with a dependency update, which is against the rules. It doesn't provide a specific suggestion or point out a specific issue.
4. src/index.js:136
- Draft comment:
Logical assignment of 'parser' for .vue/.svelte files uses ||=. Confirm that this pattern is supported in your target Node versions. - 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.
5. .eslintrc.js:8
- Draft comment:
Using 'latest' for ecmaVersion might lead to inconsistent behavior across environments. Consider specifying a fixed numeric version for better reproducibility. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 30% vs. threshold = 50%
Using 'latest' in build configurations can indeed cause reproducibility issues as new ECMAScript versions are released. This is a real concern in build tooling. The suggestion to use a fixed version is technically correct and follows good practices. However, this is a relatively minor configuration issue that is unlikely to cause immediate problems.
The comment might be too nitpicky for a PR review - ESLint's 'latest' is fairly stable and changes to ECMAScript versions are infrequent. Also, the team might have intentionally chosen 'latest' to automatically stay up to date.
While the concern is valid, this kind of configuration detail might be better discussed in a team standards document rather than a PR comment. The current change is unlikely to cause real issues.
Delete the comment. While technically correct, it's not important enough to block or modify the PR for this.
6. .github/dependabot.yml:1
- Draft comment:
Dependabot configuration has been removed. Ensure that dependency updates are managed by another mechanism or are handled manually. - 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.
7. .github/workflows/ci.yml:7
- Draft comment:
Good improvement: added concurrency settings to cancel in-progress runs and updated Node.js versions along with yarn caching. Verify that Node.js v22 and yarn v4 configurations are fully supported. - Reason this comment was not posted:
Confidence changes required:10%<= threshold50%
None
8. .nvmrc:1
- Draft comment:
Node version updated to v20.19.0. Confirm that all tools and dependencies are compatible with Node 20. - Reason this comment was not posted:
Confidence changes required:10%<= threshold50%
None
9. .yarnrc.yml:5
- Draft comment:
Ensure that the specified yarn plugin URL and checksum are maintained and trusted from a security perspective. - Reason this comment was not posted:
Confidence changes required:30%<= threshold50%
None
10. package-scripts.js:55
- Draft comment:
Including 'clean-pkg-json' before publishing on release is a good practice. Double-check that it doesn't remove or alter necessary package metadata. - Reason this comment was not posted:
Confidence changes required:20%<= threshold50%
None
11. package.json:87
- Draft comment:
The dependency versions have been comprehensively updated. Ensure that these changes do not introduce breaking changes, especially with major version bumps. - Reason this comment was not posted:
Confidence changes required:20%<= threshold50%
None
12. src/__tests__/index.js:8
- Draft comment:
Minor formatting adjustments in import statements are fine. - Reason this comment was not posted:
Confidence changes required:10%<= threshold50%
None
13. src/__tests__/utils.js:187
- Draft comment:
The test cases for getOptionsForFormatting are comprehensive and clear. - Reason this comment was not posted:
Confidence changes required:10%<= threshold50%
None
14. src/index.js:162
- Draft comment:
In createPrettify, when 'param' is not a string, 'messages' is set to param.text. This appears to be a bug; it likely should assign 'param.messages' instead. - Reason this comment was not posted:
Comment was on unchanged code.
15. src/index.js:217
- Draft comment:
Consider using a helper (e.g., an 'omit' utility) to remove properties from eslintConfig for clarity, rather than deleting each key manually. - Reason this comment was not posted:
Confidence changes required:30%<= threshold50%
None
16. README.md:268
- Draft comment:
Typo found: 'wont' should be corrected to 'won't'. - 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_prwTLrrMPhtGpJVp
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
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.
👍 Looks good to me! Incremental review on 399a57c in 3 minutes and 32 seconds
More details
- Looked at
649lines of code in19files - Skipped
0files when reviewing. - Skipped posting
9drafted comments based on config settings.
1. src/index.js:160
- Draft comment:
Potential bug: In function createPrettify, when param is not a string, the code assigns messages = param.text. It likely should be messages = param.messages to correctly capture ESLint messages. Please verify. - Reason this comment was not posted:
Comment was on unchanged code.
2. .eslintrc.js:8
- Draft comment:
Using 'ecmaVersion: "latest"' is acceptable if your toolchain supports it, but verify compatibility with environments expecting a numeric value. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%
None
3. .github/dependabot.yml:1
- Draft comment:
Dependabot configuration was removed. Confirm that dependency updates are managed elsewhere (especially during a Yarn v4 migration). - 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.
4. .husky/pre-commit:1
- Draft comment:
The Husky pre-commit hook file was removed. Ensure that its removal in favor of simple-git-hooks is intentional and update documentation if needed. - 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.
5. package-scripts.js:55
- Draft comment:
Including 'clean-pkg-json' in the release script is a good safeguard before publishing. Please ensure its behavior aligns with your release process. - Reason this comment was not posted:
Confidence changes required:30%<= threshold50%
None
6. package.json:30
- Draft comment:
The preinstall script enforces npm ('npx only-allow npm'), which conflicts with migrating to Yarn v4. Update this check to enforce Yarn or remove it. - 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 addition of npm enforcement is a deliberate choice by the maintainers. There's no evidence in the diff or context that suggests this project intends to migrate to Yarn. The comment is speculative about a potential future migration. Package manager choice is a project-level decision that should be respected.
Maybe there's an existing discussion or issue about migrating to Yarn that I'm not aware of. The comment author might have additional context.
Even if there were discussions about Yarn, the current PR explicitly adds npm enforcement, showing it's the current intended choice. Speculative comments about future changes aren't helpful.
Delete the comment as it's speculative and questions a deliberate maintainer decision without strong evidence that it's incorrect.
7. src/index.js:161
- Draft comment:
Potential bug: In createPrettify, when the input parameter is not a string, 'messages' is assigned from 'param.text'. Likely this should use 'param.messages' instead. - Reason this comment was not posted:
Comment was on unchanged code.
8. src/index.js:285
- Draft comment:
Review the default setting in getESLintApiOptions: 'ignore: eslintConfig.ignore || true' will always return true, even if eslintConfig.ignore is explicitly set to false. Verify if this behavior is intended. - 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.
9. README.md:267
- Draft comment:
Typographical error: "wont" should be written as "won't" in the sentence "And theeslint --fixwont change it back." - 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_WFWhXn14Qb6UBvfs
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1171 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 2 2
Lines 303 303
Branches 84 84
=========================================
Hits 303 303 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
👍 Looks good to me! Incremental review on eba2e9a in 1 minute and 32 seconds
More details
- Looked at
645lines of code in19files - Skipped
0files when reviewing. - Skipped posting
9drafted comments based on config settings.
1. src/index.js:286
- Draft comment:
Using the||operator to default values in getESLintApiOptions can cause issues if a falsy value (like false or 0) is intended. Consider using nullish coalescing (??) to avoid unintentional overrides. - 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. src/index.js:241
- Draft comment:
Avoid usingawait reportin array destructuring since report is already awaited. - 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/index.js:162
- Draft comment:
Check the assignment for messages in createPrettify: usingparam.textinstead of what likely should beparam.messages. - Reason this comment was not posted:
Comment was on unchanged code.
4. src/index.js:335
- Draft comment:
Consider handling module resolution failures more gracefully. The current fallback in getModulePath logs a debug message and falls back to require.resolve; ensure this behavior is documented. - Reason this comment was not posted:
Confidence changes required:30%<= threshold50%
None
5. src/index.js:159
- Draft comment:
In the createPrettify function, when the parameter is an object, the code assignsmessages = param.text. This seems like a bug—it probably should useparam.messagesto capture ESLint messages. - Reason this comment was not posted:
Comment was on unchanged code.
6. src/index.js:66
- Draft comment:
When defaulting text with getTextFromFilePath(filePath), there's no check to ensure that filePath is provided. Consider validating that either a text or a valid filePath is supplied to avoid errors from fs.readFileSync(undefined, 'utf8'). - 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.
7. src/index.js:195
- Draft comment:
The createEslintFix function mutates the passed eslintConfig by setting an overrideConfig property and then deleting several keys. This mutation could lead to side effects. It's recommended to clone the configuration object before modifying it. - 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.
8. src/index.js:285
- Draft comment:
In getESLintApiOptions, the default assignmentignore: eslintConfig.ignore || truealways results in true if eslintConfig.ignore is falsy. Consider revisiting this logic, perhaps using a nullish coalescing operator, to ensure the intended default behavior. - 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.
9. src/index.js:350
- Draft comment:
The module contains several complex functions with multiple responsibilities. Consider refactoring parts of src/index.js into smaller, more focused modules to improve readability and maintainability. - 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_mJvzMIYADSLtyTOZ
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
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.
👍 Looks good to me! Incremental review on 7be8407 in 1 minute and 20 seconds
More details
- Looked at
645lines of code in19files - Skipped
0files when reviewing. - Skipped posting
6drafted comments based on config settings.
1. src/index.js:159
- Draft comment:
In createPrettify, when param is not a string, you setmessages = param.text. This seems off – likely you meant to useparam.messagesto capture lint messages. - Reason this comment was not posted:
Comment was on unchanged code.
2. src/index.js:204
- Draft comment:
In createEslintFix, the ESLint config object is mutated by deleting several properties. Consider cloning the config to avoid side effects. - 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/index.js:241
- Draft comment:
The destructuring assignment usesawait reporteven thoughreportwas already awaited on line 231. Consider removing the redundantawait. - 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.
4. src/index.js:286
- Draft comment:
Using the logical OR operator here (eslintConfig.ignore || true) will always default to true even if ignore is explicitly set to false. Consider using the nullish coalescing operator (??) to preserve false values. - 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.
5. src/index.js:217
- Draft comment:
The function deletes several properties from the ESLint config (e.g., rules, parser, globals). This mutates the object. Consider cloning the config before modifying it if these properties might be needed later. - 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.
6. src/index.js:67
- Draft comment:
When 'text' is not provided, the code immediately calls getTextFromFilePath(filePath) without checking if filePath is defined. Ensure that either a valid filePath or text is provided to avoid runtime errors. - 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_JEseVd8DlCdwf1i9
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
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.
👍 Looks good to me! Incremental review on 0af0c33 in 1 minute and 38 seconds
More details
- Looked at
645lines of code in19files - Skipped
0files when reviewing. - Skipped posting
13drafted comments based on config settings.
1. src/index.js:217
- Draft comment:
Consider cloning the eslintConfig before deleting its keys to avoid side effects if the object is reused later. - 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. src/index.js:135
- Draft comment:
Good use of ||= for setting default ESLint parsers for TS, Vue, and Svelte files. Ensure that this syntax is acceptable in your targeted Node versions. - Reason this comment was not posted:
Confidence changes required:30%<= threshold50%
None
3. package-scripts.js:55
- Draft comment:
Using a series call to run 'clean-pkg-json' before publishing is clear; ensure that the simple-git-hooks setup works as expected. - Reason this comment was not posted:
Confidence changes required:20%<= threshold50%
None
4. .eslintrc.js:8
- Draft comment:
ECMA version is now set to 'latest'. Using a string here can be less explicit—consider using the numeric version to guarantee consistent behavior. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%
None
5. .github/dependabot.yml:1
- Draft comment:
Dependabot configuration has been removed. Ensure this is intentional and that dependency updates are handled by an alternative process. - 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.
6. .husky/pre-commit:1
- Draft comment:
The Husky pre-commit hook has been removed in favor of simple-git-hooks. Confirm that this change preserves your intended pre-commit validations. - 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.
7. .nvmrc:1
- Draft comment:
nvmrc now specifies 'v20.19.0'. Ensure this version aligns with the engines specified in package.json and the CI matrix. - 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%
This is a "verify/ensure" type comment that asks the author to double-check something. It's asking for verification across multiple files that we can't see. The rules explicitly state not to make comments that ask authors to verify things or that require cross-file context.
The Node.js version change could potentially cause breaking changes, so maybe this verification is important enough to keep?
While Node.js version compatibility is important, the rules clearly state not to make comments that ask authors to verify things. If there was an actual version mismatch, that would be caught by CI.
Delete this comment as it violates the rules against asking authors to verify things and against cross-file issues.
8. package-scripts.js:55
- Draft comment:
The release script now runs 'clean-pkg-json' before publishing. Verify that 'clean-pkg-json' is installed and correctly configured as part of your release process. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%
None
9. src/index.js:136
- Draft comment:
The use of the logical OR assignment (||=) for setting the ESLint parser requires Node 16+. Confirm that this meets your support requirements. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%
None
10. src/index.js:217
- Draft comment:
The function createEslintFix mutates the eslintConfig by deleting several properties. Consider cloning the config to avoid side effects if it is reused elsewhere. - 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.
11. src/index.js:241
- Draft comment:
Destructuring the report with 'const [{ output = text, messages }] = await report;' assumes the report array is non-empty. It might be safer to add a check for an empty result. - 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.
12. README.md:110
- Draft comment:
Typo: Duplicate 'the' found in the fallbackPrettierOptions section. Consider changing "based on the theeslintConfig" to "based on theeslintConfig". - 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.
13. README.md:268
- Draft comment:
Typo: The word 'wont' should be corrected to "won't" in the sentence "And theeslint --fixwont change it back.". - 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_3OaILoUDhyzyF5wz
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
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.
❌ Changes requested. Incremental review on c56e566 in 2 minutes and 26 seconds
More details
- Looked at
648lines of code in19files - Skipped
0files when reviewing. - Skipped posting
8drafted comments based on config settings.
1. .eslintrc.js:8
- Draft comment:
Using ecmaVersion: 'latest' is valid in newer ESLint versions. Verify that downstream tools/configs handle the string value as expected. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
2. .github/dependabot.yml:1
- Draft comment:
Dependabot configuration was completely removed. Confirm that dependency updates will be handled via another mechanism. - 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. package-scripts.js:55
- Draft comment:
The release script now chains 'clean-pkg-json' before changeset publish. Ensure that the 'clean-pkg-json' script is defined. - 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 a script is defined, which falls under the rule of not asking the author to ensure something. It does not provide a specific suggestion or point out a specific issue with the code.
4. .nvmrc:1
- Draft comment:
The Node version in .nvmrc has been bumped from 16.20.2 to v20.19.0. Verify that all environments support this change. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to verify that all environments support the change in Node version. This falls under the rule of not asking the author to ensure compatibility or verify intentions. Therefore, it should be removed.
5. src/index.js:138
- Draft comment:
Logical OR assignment is used to set the parser for TypeScript files. Ensure that your deployment environment supports Node’s logical assignment operators. - 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.
6. src/index.js:288
- Draft comment:
Using '||' for default values in getESLintApiOptions overrides explicit false settings. Consider a ternary check to preserve false values. - 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.
7. src/index.js:151
- Draft comment:
The use of 'await' with createEslintFix is unnecessary as the function is synchronous (it returns an async function). - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%
None
8. src/index.js:219
- Draft comment:
createEslintFix modifies eslintConfig in place by deleting several properties. Ensure this side effect is acceptable. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
Workflow ID: wflow_Xmwsdoj8dizLSHew
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
|
Hello, this PR appears to have broken https://github.com/prettier/prettier-eslint-cli. See #1175 |
commit: |
Important
Update dependencies, configurations, and CI/CD workflows for improved development and build processes.
@typescript-eslint/parserto^6.21.0,eslintto^8.57.1,prettierto^3.5.3, and other dependencies inpackage.json.clean-pkg-json,only-allow,prettier-plugin-pkg,simple-git-hookstodevDependencies.huskyandtypescriptfromdevDependencies..editorconfigfor consistent coding styles..prettierrc.jswith.prettierrc.jsonand update configuration..simple-git-hooks.jsonfor git hooks configuration..eslintrc.jsto useecmaVersion: 'latest'..nvmrctov20.19.0.ci.yml,release.yml, andstale.ymlto useactions/checkout@v4,actions/setup-node@v4, andcodecov/codecov-action@v5..github/dependabot.ymland.husky/pre-commit.package-scripts.jsto includeclean-pkg-jsonin the release script.README.mdfor minor formatting changes.This description was created by
for c56e566. It will automatically update as commits are pushed.