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.

Sign up for GitHub

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

fix: Use require.resolve when yarn v2 is detected #15623

Closed
wants to merge 18 commits into from
Closed

fix: Use require.resolve when yarn v2 is detected #15623

Changes from 16 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
ac2f274
fix: Use require.resolve when yarn v2 is detected
blake-transcend Mar 22, 2021
e0dea49
Update resolve.js
blake-transcend Mar 22, 2021
e31ce74
Merge branch 'develop' into issue-8008-typescript-plugins-pnp
blake-transcend Mar 23, 2021
f457d77
Merge branch 'develop' into issue-8008-typescript-plugins-pnp
blake-transcend Mar 23, 2021
8120618
Merge branch 'develop' into issue-8008-typescript-plugins-pnp
blake-transcend Mar 24, 2021
051aec4
Merge branch 'develop' into issue-8008-typescript-plugins-pnp
blake-transcend Mar 30, 2021
89c245b
Update resolve.js
blake-transcend Apr 1, 2021
89997d4
Update resolve.js
blake-transcend Apr 1, 2021
4e47428
Update resolve.js
blake-transcend Apr 1, 2021
67cf414
Merge branch 'develop' into issue-8008-typescript-plugins-pnp
blake-transcend Apr 1, 2021
a056378
Merge branch 'develop' into issue-8008-typescript-plugins-pnp
blake-transcend Apr 2, 2021
d889411
Merge branch 'develop' into issue-8008-typescript-plugins-pnp
blake-transcend Apr 5, 2021
89d0252
Merge branch 'develop' into issue-8008-typescript-plugins-pnp
blake-transcend Apr 5, 2021
2529c82
Merge branch 'develop' into issue-8008-typescript-plugins-pnp
blake-transcend Apr 5, 2021
7ce42cb
Merge branch 'develop' into issue-8008-typescript-plugins-pnp
blake-transcend Apr 6, 2021
c135cda
Merge branch 'develop' into issue-8008-typescript-plugins-pnp
blake-transcend Apr 6, 2021
b8c5c6d
add 4_yarn_v2_spec
flotwig Apr 8, 2021
2a58263
update spec to use `yarn node` to launch cli
flotwig Apr 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions packages/server/lib/util/resolve.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
const resolve = require('resolve')
const env = require('./env')
const debug = require('debug')('cypress:server:plugins')

Expand All @@ -15,13 +14,9 @@ module.exports = {
}

try {
const options = {
basedir: projectRoot,
}
debug('resolving typescript for %o', projectRoot)

debug('resolving typescript with options %o', options)

const resolved = resolve.sync('typescript', options)
const resolved = require.resolve('typescript', { paths: [projectRoot] })
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the problem is that require.resolve here is still not the patched require.resolve that Yarn v2 expects, since we are running Cypress inside of Cypress's own Electron-Node instance.

Maybe the solution is to fall back to trying to use @yarnpkg/pnp to discover the path if require.resolve('typescript') fails? https://yarnpkg.com/advanced/pnpapi#resolvetounqualified

Copy link
Author

Choose a reason for hiding this comment

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

This code actually works fine for me, my coworkers, and our CI. Could the cause be that your e2e test isn't executing cypress from yarn like you're supposed to (yarn cypress ...)? I believe that's where it injects its custom resolver. It would also probably be pretty difficult to detect which package manager you are using if you're executing outside of your package manager's environment

Copy link
Contributor

@flotwig flotwig Apr 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Oh, I didn't realize yarn v2 required launching stuff like that, it will probably pass once the test is updated then

Copy link
Contributor

Choose a reason for hiding this comment

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

@blake-transcend I updated the spec to use yarn node to launch cypress run via the CLI, but it still seems to fail with that same error. Since I'm not super familiar with how to test this (even manually), do you want to take a try at writing a test for this?

Copy link
Author

Choose a reason for hiding this comment

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

Sure I can try with your test


debug('resolved typescript %s', resolved)

Expand Down