Skip to content
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

Avoid ESLint path resolution errors on Windows #469

Merged
merged 21 commits into from Nov 15, 2023

Conversation

karlhorky
Copy link
Member

@karlhorky karlhorky commented Nov 14, 2023

Avoid ESLint path resolution errors on Windows, such as this test run failure:

> @upleveled/preflight@4.0.0 test D:\a\preflight\preflight
> tsdx test "--ci" "--maxWorkers=2"

FAIL __tests__/e2e.test.ts (69.746 s)
  × Passes in the react-passing test project (9630 ms)
  × Passes in the next-js-passing test project (86[14](https://github.com/upleveled/preflight/actions/runs/6842461034/job/18603950750#step:15:15) ms)

  ● Passes in the react-passing test project

    expect(received).toMatchSnapshot()

    Snapshot name: `Passes in the react-passing test project 1`

    - Snapshot  - 1
    + Received  + 0

    @@ -1,8 +1,7 @@
      "🚀 UpLeveled Preflight
      [COMPLETED] All changes committed to Git
    - [COMPLETED] ESLint
      [COMPLETED] ESLint config is latest version
      [COMPLETED] GitHub repo has deployed project link under About
      [COMPLETED] No dependencies without types
      [COMPLETED] No dependency problems
      [COMPLETED] No extraneous files committed to Git

      104 |   );
      105 |
    > 106 |   expect(sortStdoutAndStripVersionNumber(stdout)).toMatchSnapshot();
          |                                                   ^
      107 |   expect(stderr.replace(/^\(node:\d+\) /, '')).toMatchSnapshot();
      108 | }, 30000);
      109 |

      at Object.<anonymous> (__tests__/e2e.test.ts:106:51)

  ● Passes in the react-passing test project
      [COMPLETED] ESLint config is latest version
      [COMPLETED] GitHub repo has deployed project link under About
      [COMPLETED] Next.js project has sharp installed
      [COMPLETED] No dependencies without types
      [COMPLETED] No dependency problems

      1[16](https://github.com/upleveled/preflight/actions/runs/6842461034/job/18603950750#step:15:17) |   );
      1[17](https://github.com/upleveled/preflight/actions/runs/6842461034/job/18603950750#step:15:18) |
    > 1[18](https://github.com/upleveled/preflight/actions/runs/6842461034/job/18603950750#step:15:19) |   expect(sortStdoutAndStripVersionNumber(stdout)).toMatchSnapshot();
          |                                                   ^
      1[19](https://github.com/upleveled/preflight/actions/runs/6842461034/job/18603950750#step:15:20) |   expect(stderr.replace(/^\(node:\d+\) /, '')).toMatchSnapshot();
      1[20](https://github.com/upleveled/preflight/actions/runs/6842461034/job/18603950750#step:15:21) | }, 45000);
      1[21](https://github.com/upleveled/preflight/actions/runs/6842461034/job/18603950750#step:15:22) |

      at Object.<anonymous> (__tests__/e2e.test.ts:118:51)

  ● Passes in the next-js-passing test project

    expect(received).toMatchSnapshot()

    Snapshot name: `Passes in the next-js-passing test project 2`

    - Snapshot  -  1
    + Received  + 17

    - ""
    + "[FAILED] Command failed with exit code 2: pnpm eslint . --max-warnings 0 --format json
    +
    + Oops! Something went wrong! :(
    +
    + ESLint: 8.53.0
    +
    + Error: Cannot find package 'D:\a\preflight\preflight\__tests__\fixtures\__temp\next-js-passing\node_modules\.pnpm\eslint-config-upleveled@7.0.0_@babel+eslint-parser@7.23.3_@next+eslint-plugin-next@14.0.2_@ty_xvhbeu5qc6hlxssq5gmtnagbti\node_modules\eslint-plugin-jsx-expressions\package.json' imported from D:\a\preflight\preflight\__tests__\fixtures\__temp\next-js-passing\node_modules\.pnpm\eslint-config-upleveled@7.0.0_@babel+eslint-parser@7.23.3_@next+eslint-plugin-next@14.0.2_@ty_xvhbeu5qc6hlxssq5gmtnagbti\node_modules\eslint-config-upleveled\index.js
    + Did you mean to import eslint-plugin-jsx-expressions@1.3.1_@typescript-eslint+parser@6.10.0_eslint@8.53.0_typescript@5.2.2/node_modules/eslint-plugin-jsx-expressions/dist/index.js?
    +     at legacyMainResolve (node:internal/modules/esm/resolve:189:26)
    +     at packageResolve (node:internal/modules/esm/resolve:776:14)
    +     at moduleResolve (node:internal/modules/esm/resolve:838:20)
    +     at defaultResolve (node:internal/modules/esm/resolve:1043:11)
    +     at ModuleLoader.defaultResolve (node:internal/modules/esm/loader:383:12)
    +     at ModuleLoader.resolve (node:internal/modules/esm/loader:352:25)
    +     at ModuleLoader.getModuleJob (node:internal/modules/esm/loader:[22](https://github.com/upleveled/preflight/actions/runs/6842461034/job/18603950750#step:15:23)8:38)
    +     at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:85:39)
    +     at link (node:internal/modules/esm/module_job:84:[36](https://github.com/upleveled/preflight/actions/runs/6842461034/job/18603950750#step:15:37))"

      117 |
      118 |   expect(sortStdoutAndStripVersionNumber(stdout)).toMatchSnapshot();
    > 119 |   expect(stderr.replace(/^\(node:\d+\) /, '')).toMatchSnapshot();
          |                                                ^
      120 | }, 45000);
      121 |

      at Object.<anonymous> (__tests__/e2e.test.ts:119:48)

 › 4 snapshots failed.
Snapshot Summary
 › 4 snapshots failed from 1 test suite. Inspect your code changes or re-run jest with `-u` to update them.

Test Suites: 1 failed, 1 total
Tests:       2 failed, 2 total
Snapshots:   4 failed, 4 total
Time:        70.[43](https://github.com/upleveled/preflight/actions/runs/6842461034/job/18603950750#step:15:44)6 s
Ran all test suites.
 ELIFECYCLE  Test failed. See above for more details.
Error: Process completed with exit code 1.

Funny enough, the file that it cannot resolve above (D:\a\preflight\preflight\__tests__\fixtures\__temp\next-js-passing\node_modules\.pnpm\eslint-config-upleveled@7.0.0_@babel+eslint-parser@7.23.3_@next+eslint-plugin-next@14.0.2_@ty_xvhbeu5qc6hlxssq5gmtnagbti\node_modules\eslint-plugin-jsx-expressions\package.json) actually does exist on disk:

const { stdout: stdout1 } = await execaCommand('dir', {
  cwd: `D:\\a\\preflight\\preflight\\__tests__\\fixtures\\__temp\\react-passing\\node_modules\\.pnpm\\eslint-config-upleveled@7.0.0_@babel+eslint-parser@7.23.3_@next+eslint-plugin-next@14.0.2_@ty_xvhbeu5qc6hlxssq5gmtnagbti\\node_modules\\eslint-plugin-jsx-expressions\\`,
});
console.log(stdout1);
// README.md  dist  docs  node_modules  package.json

So seems like a bug in ESLint's package resolution, opened an issue

Copy link

github-actions bot commented Nov 14, 2023

size-limit report 📦

Path Size
dist/preflight.esm.js 7.4 KB (0%)

karlhorky added a commit to upleveled/eslint-config-upleveled that referenced this pull request Nov 14, 2023
To work around package resolution errors

Alternative to upleveled/preflight#469
@karlhorky
Copy link
Member Author

karlhorky commented Nov 15, 2023

So the reason that the node-linker=hoisted in .npmrc in b2c193b works is that ESLint and Node.js have a problem with long paths (paths over the 260 character MAX_PATH limit on Windows):

So if we reduce the path length, then that would be another solution

@karlhorky
Copy link
Member Author

Using tmpdir() from node:os is another option, and doesn't require us to change the way pnpm installs on Windows.

Comment on lines -82 to +91
// 5 minute timeout for pnpm installation inside test repos
300000,
// 7.5 minute timeout for pnpm installation inside test repos
450000,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have to increase the timeout because it takes longer for pnpm to install on Windows in tmpdir() apparently

@karlhorky karlhorky merged commit 8012268 into main Nov 15, 2023
6 checks passed
@karlhorky karlhorky deleted the avoid-eslint-path-resolution-error branch November 15, 2023 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants