-
-
Notifications
You must be signed in to change notification settings - Fork 74
fix: require entry types, add module-sync entry
#417
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: a9bfe72 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 |
|
Report too large to display inline |
|
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 📦
|
commit: |
WalkthroughThis pull request introduces several configuration and declaration changes. A new patch file is added to improve the Changes
Sequence Diagram(s)sequenceDiagram
participant ESLint as ESLint Engine
participant Config as ESLint Config Loader
ESLint->>Config: Request configuration
Config-->>Config: Check Node version condition
alt Node version ≤ 16
Config-->>ESLint: Return empty configuration {}
else Node version > 16
Config-->>ESLint: Return complete configuration object
end
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (8)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 updates configuration to fix the handling of require entry types and adds support for a module-sync entry.
- Updates the Yarn configuration with a specific yarnPath.
- Adjusts the ESLint configuration to conditionally run based on the Node major version.
- Adds a new module export file, updates the release workflow permissions, and introduces a changeset note.
Reviewed Changes
Copilot reviewed 8 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/pnp/.yarnrc.yml | Adds a yarnPath setting for a specific Yarn release version. |
| tests/importXResolverV3/eslint.config.cjs | Updates the Node version condition and removes an inline comment. |
| index.d.cts | Introduces a new module export file. |
| .github/workflows/release.yml | Adds necessary permissions for the release workflow. |
| .changeset/gold-rabbits-walk.md | Updates the changeset note to reflect the new fixes and module-sync entry. |
Files not reviewed (5)
- .codesandbox/ci.json: Language not supported
- package.json: Language not supported
- tests/pnp/.pnp.cjs: Language not supported
- tests/pnp/.pnp.loader.mjs: Language not supported
- tests/pnp/package.json: Language not supported
Comments suppressed due to low confidence (1)
tests/importXResolverV3/eslint.config.cjs:16
- [nitpick] Consider extracting the Node major version into a well-named constant (e.g., 'nodeMajor') before the conditional check to improve clarity.
process.versions.node.split('.')[0] <= 16
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.
Actionable comments posted: 0
🧹 Nitpick comments (8)
tests/pnp/.pnp.cjs (4)
1462-1476: Clarification for TS Error Suppression in Hunk 3The comments for the
@ts-expect-errordirectives now include the note “reason TBS”. This improves clarity by indicating that the error suppression is intentional and explained (albeit briefly). For long-term maintainability, consider replacingTBSwith a more descriptive explanation when further details become available.
1890-1904: Enhanced TS Error Directive in Hunk 4The modifications update the error suppression comments in both
watchandwatchFilemethods to include “reason TBS”. This consistency aids in understanding why TypeScript errors are ignored. It might be beneficial to expandTBS(e.g., “to be specified”) to a more detailed rationale once the underlying issue is fully addressed.
2593-2606: Consistent Update of TS Error Suppression in Hunk 5Here, the updated
@ts-expect-error - reason TBScomments are applied both in thebaseFs.watchcall and in the mount file system callback. This consistency is good; however, consider providing additional context (beyond “TBS”) if the error suppression becomes a recurring source of questions from future maintainers.
2610-2616: Harmonized TS Error Comment in Hunk 6The change in this hunk also updates the
@ts-expect-errorcomment to include “reason TBS”. This aligns with the other changes for similar functions throughout the file. For better long-term documentation, it may be worthwhile to eventually elaborate on the cause necessitating the error suppression..changeset/gold-rabbits-walk.md (1)
1-6: Document Patch for ESLint Import Resolver
This changeset documents the patch for fixing therequireentry types and adding a newmodule-syncentry. The note clearly reflects the PR objective and provides context for the changes. Consider expanding on the “reason” behind the changes if additional details could help future maintainers.tests/pnp/.pnp.loader.mjs (2)
896-902: Clarify TS Error Suppression inwatchMethod
The modified TS error suppression comment now reads// @ts-expect-error - reason TBSWhile it is good to include a reason, “TBS” (To Be Specified) is rather vague. Consider providing a more detailed explanation of the expected TypeScript error and why it can be safely ignored, to help future maintainers understand the context.
904-910: Clarify TS Error Suppression inwatchFileMethod
Similarly, thewatchFilemethod now includes a TS error suppression comment with “reason TBS.” It would be beneficial to replace “TBS” with a more descriptive note that explains the underlying issue and justification for suppressing the error.package.json (1)
60-60: Validate Prepare Script Chaining:
The updated"prepare"script now chains additional commands to change directories intotests/nestedPackageJsonand runyarn, with a fallback (|| exit 0). Please review that this chaining (using&&and||) behaves as intended—especially considering that a failure inyarnwill trigger an immediate exit with status 0, potentially masking errors during setup. Confirm whether this silent exit on error is acceptable for your workflow or if you’d like to handle failures more robustly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
tests/nestedPackageJson/yarn.lockis excluded by!**/yarn.lock,!**/*.locktests/pnp/.yarn/releases/yarn-4.8.0.cjsis excluded by!**/.yarn/**yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
.changeset/gold-rabbits-walk.md(1 hunks).codesandbox/ci.json(1 hunks).github/workflows/release.yml(1 hunks)index.d.cts(1 hunks)package.json(2 hunks)tests/importXResolverV3/eslint.config.cjs(0 hunks)tests/pnp/.pnp.cjs(6 hunks)tests/pnp/.pnp.loader.mjs(2 hunks)tests/pnp/.yarnrc.yml(1 hunks)tests/pnp/package.json(1 hunks)
💤 Files with no reviewable changes (1)
- tests/importXResolverV3/eslint.config.cjs
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Lint and Test with Node.js 22 on windows-latest
🔇 Additional comments (8)
.codesandbox/ci.json (1)
1-5: Simplification via Removal of Unused "packages" Key
The removal of the"packages"property streamlines this JSON configuration. Make sure that no downstream tooling or integrations are still relying on this key. If it's no longer used, this change is appropriate..github/workflows/release.yml (1)
13-17: Addition of Explicit Workflow Permissions
The introduction of thepermissionsblock withcontents: write,id-token: write, andpull-requests: writeclearly delineates the access scope for the release workflow. Please verify that these permissions are scoped as narrowly as needed and are aligned with your security policies.tests/pnp/.pnp.cjs (2)
27-34: Configuration Dependency Addition in Hunk 1The dependency list now includes a new entry
["pnp", "workspace:."], which fulfills the PR objective regarding fixing therequireentry types. Please verify that the ordering of dependencies remains consistent with other configuration contexts in the project.
46-53: Consistent Package Dependency Addition in Hunk 2In this block, the dependency entry
["pnp", "workspace:."]is similarly added alongside["lodash.zip", "npm:4.2.0"]. Ensure that this new entry is correctly referenced wherever similar dependency configurations are expected.tests/pnp/package.json (1)
4-4: Update Yarn Version in Package Manager Field
ThepackageManagerfield is updated to"yarn@4.8.0", which aligns with the corresponding Yarn binary path in the.yarnrc.ymlfile. Please verify that this new version is fully compatible with your project’s requirements and that there are no residual references to the old version.index.d.cts (1)
1-3: New Declaration File for ESLint Import Resolver
This new declaration file imports the default export from./lib/index.jsand re‑exports it using CommonJS syntax (export =). This implementation correctly supports the updatedrequireentry types and facilitates TypeScript’s type resolution.tests/pnp/.yarnrc.yml (1)
4-5: Specify Yarn Binary Path for Yarn v4.8.0
The addition of the lineyarnPath: .yarn/releases/yarn-4.8.0.cjsensures that the correct Yarn version is used in the project. This change is consistent with the update in
package.json. Please verify that the provided path is accurate and that the binary is correctly installed at that location.package.json (1)
21-37: Revised Exports Structure Verification:
The new"exports"block now clearly distinguishes resolution strategies by defining separate entries for"import","module-sync","require", and"default". Please verify that the new"require"entry’s types path ("./index.d.cts") correctly points to the intended declaration file and that it integrates well with TypeScript’s module resolution.
@coderabbitai You need to install dependencies with yarn + corepack. |
| datasource | package | from | to | | ---------- | --------------------------------- | ----- | ----- | | npm | eslint-import-resolver-typescript | 4.2.2 | 4.3.1 | ## [v4.3.1](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#431) ##### Patch Changes - [#425](import-js/eslint-import-resolver-typescript#425) [`2ced0ba`](import-js/eslint-import-resolver-typescript@2ced0ba) Thanks [@JounQin](https://github.com/JounQin)! - chore: bump `unrs-resolver` to v1.3.3 ## [v4.3.0](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#430) ##### Minor Changes - [#423](import-js/eslint-import-resolver-typescript#423) [`2fcb947`](import-js/eslint-import-resolver-typescript@2fcb947) Thanks [@JounQin](https://github.com/JounQin)! - feat: throw error on malformed `tsconfig` reference ## [v4.2.7](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#427) ##### Patch Changes - [`aeb558f`](import-js/eslint-import-resolver-typescript@aeb558f) Thanks [@JounQin](https://github.com/JounQin)! - fix: add missing `index.d.cts` file ## [v4.2.6](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#426) ##### Patch Changes - [#417](import-js/eslint-import-resolver-typescript#417) [`c3f678b`](import-js/eslint-import-resolver-typescript@c3f678b) Thanks [@JounQin](https://github.com/JounQin)! - fix: `require` entry types, add `module-sync` entry ## [v4.2.5](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#425) ##### Patch Changes - [#410](import-js/eslint-import-resolver-typescript#410) [`ec59d22`](import-js/eslint-import-resolver-typescript@ec59d22) Thanks [@JounQin](https://github.com/JounQin)! - fix: absolute path aliasing should not be skipped ## [v4.2.4](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#424) ##### Patch Changes - [#407](import-js/eslint-import-resolver-typescript#407) [`6b183ff`](import-js/eslint-import-resolver-typescript@6b183ff) Thanks [@JounQin](https://github.com/JounQin)! - chore: migrate to rebranding `unrs-resolver` with new targets supported: - `i686-pc-windows-msvc` - `armv7-unknown-linux-musleabihf` - `powerpc64le-unknown-linux-gnu` - `s390x-unknown-linux-gnu` ## [v4.2.3](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#423) ##### Patch Changes - [#402](import-js/eslint-import-resolver-typescript#402) [`f21bf15`](import-js/eslint-import-resolver-typescript@f21bf15) Thanks [@SunsetTechuila](https://github.com/SunsetTechuila)! - fix: don't resolve not implemented node modules in `bun` `is-bun-module` is marked as `dependency`, again, for correctness, see [`isBunImplementedNodeModule`](https://github.com/SunsetTechuila/is-bun-module#isbunimplementednodemodulemodulename-bunversion) for more details For `Bun` users: you don't need to install `is-bun-module` any more but `bun: true` option is still required if you're running without `bun --bun` nor [`run#bun`](https://bun.sh/docs/runtime/bunfig#run-bun-auto-alias-node-to-bun) enabled
| datasource | package | from | to | | ---------- | --------------------------------- | ----- | ----- | | npm | eslint-import-resolver-typescript | 4.2.2 | 4.3.1 | ## [v4.3.1](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#431) ##### Patch Changes - [#425](import-js/eslint-import-resolver-typescript#425) [`2ced0ba`](import-js/eslint-import-resolver-typescript@2ced0ba) Thanks [@JounQin](https://github.com/JounQin)! - chore: bump `unrs-resolver` to v1.3.3 ## [v4.3.0](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#430) ##### Minor Changes - [#423](import-js/eslint-import-resolver-typescript#423) [`2fcb947`](import-js/eslint-import-resolver-typescript@2fcb947) Thanks [@JounQin](https://github.com/JounQin)! - feat: throw error on malformed `tsconfig` reference ## [v4.2.7](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#427) ##### Patch Changes - [`aeb558f`](import-js/eslint-import-resolver-typescript@aeb558f) Thanks [@JounQin](https://github.com/JounQin)! - fix: add missing `index.d.cts` file ## [v4.2.6](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#426) ##### Patch Changes - [#417](import-js/eslint-import-resolver-typescript#417) [`c3f678b`](import-js/eslint-import-resolver-typescript@c3f678b) Thanks [@JounQin](https://github.com/JounQin)! - fix: `require` entry types, add `module-sync` entry ## [v4.2.5](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#425) ##### Patch Changes - [#410](import-js/eslint-import-resolver-typescript#410) [`ec59d22`](import-js/eslint-import-resolver-typescript@ec59d22) Thanks [@JounQin](https://github.com/JounQin)! - fix: absolute path aliasing should not be skipped ## [v4.2.4](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#424) ##### Patch Changes - [#407](import-js/eslint-import-resolver-typescript#407) [`6b183ff`](import-js/eslint-import-resolver-typescript@6b183ff) Thanks [@JounQin](https://github.com/JounQin)! - chore: migrate to rebranding `unrs-resolver` with new targets supported: - `i686-pc-windows-msvc` - `armv7-unknown-linux-musleabihf` - `powerpc64le-unknown-linux-gnu` - `s390x-unknown-linux-gnu` ## [v4.2.3](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#423) ##### Patch Changes - [#402](import-js/eslint-import-resolver-typescript#402) [`f21bf15`](import-js/eslint-import-resolver-typescript@f21bf15) Thanks [@SunsetTechuila](https://github.com/SunsetTechuila)! - fix: don't resolve not implemented node modules in `bun` `is-bun-module` is marked as `dependency`, again, for correctness, see [`isBunImplementedNodeModule`](https://github.com/SunsetTechuila/is-bun-module#isbunimplementednodemodulemodulename-bunversion) for more details For `Bun` users: you don't need to install `is-bun-module` any more but `bun: true` option is still required if you're running without `bun --bun` nor [`run#bun`](https://bun.sh/docs/runtime/bunfig#run-bun-auto-alias-node-to-bun) enabled
https://arethetypeswrong.github.io/?p=eslint-import-resolver-typescript
Summary by CodeRabbit
New Features
Chores
.gitignoreto exclude Plug'n'Play related files from version control.