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

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

Closed
Show file tree
Hide file tree
Changes from all 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
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

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
63 changes: 63 additions & 0 deletions packages/server/test/e2e/4_yarn_v2_spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import fs from 'fs-extra'
import os from 'os'
import path from 'path'
import cp from 'child_process'
import util from 'util'
import e2e from '../support/helpers/e2e'
import Fixtures from '../support/helpers/fixtures'

const exec = async (cmd, ...args) => {
console.log(`Running "${cmd}"...`)
const ret = await util.promisify(cp.exec)(cmd, ...args)
.catch((err) => {
console.error('Error:', err)

return err
})

console.log('stdout:', ret.stdout)
ret.stderr && console.log('stderr:', ret.stderr)

return ret
}

const fixtureDir = Fixtures.path('projects/yarn-v2')
const cypressCli = path.join(__dirname, '../../../../cli/bin/cypress')

describe('e2e yarn v2', () => {
let projectDir

beforeEach(async function () {
this.timeout(240000)

// copy yarn-v2 to tmpdir so node_modules resolution won't fall back to project root
projectDir = path.join(os.tmpdir(), `cy-yarn-v2-${Date.now()}`)

await fs.mkdir(projectDir)
await fs.copy(fixtureDir, projectDir)

const projectExec = (cmd) => exec(cmd, { cwd: projectDir })

// set up and verify the yarn v2 project
// https://yarnpkg.com/getting-started/migration#step-by-step
await projectExec('yarn set version berry')
expect((await projectExec('yarn --version')).stdout).to.match(/^2\./)
await projectExec('yarn')
})

// @see https://github.com/cypress-io/cypress/pull/15623
e2e.it('can load typescript plugins files', {
snapshot: false,
command: 'yarn',
browser: 'electron',
onRun: async (run) => {
await run({
args: `node ${cypressCli} run --dev --spec=* --project=${projectDir}`.split(' '),
spawnOpts: {
cwd: projectDir,
shell: true,
},
})
},
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
nodeLinker: pnp
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
it('passes', () => {})
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// typescript dummy code
exports = (on: any, config: any) => {
return config
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"name": "yarn-v2",
"version": "1.0.0",
"main": "index.js",
"devDependencies": {
"typescript": "^4.2.4"
},
"license": "MIT"
}
34 changes: 34 additions & 0 deletions packages/server/test/support/fixtures/projects/yarn-v2/yarn.lock
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# This file is generated by running "yarn install" inside your project.
# Manual changes might be lost - proceed with caution!

__metadata:
version: 4
cacheKey: 7

typescript@^4.2.4:
version: 4.2.4
resolution: "typescript@npm:4.2.4"
bin:
tsc: bin/tsc
tsserver: bin/tsserver
checksum: edaede2fa77f56b7fba80ee624a2368ab1216e75b0434d968ccb47ab0a5e2f6d94f848b3b111c1237dd71e988cd376af26370dcdad3b94355c76e759f0dd0a1e
languageName: node
linkType: hard

"typescript@patch:typescript@^4.2.4#builtin<compat/typescript>":
version: 4.2.4
resolution: "typescript@patch:typescript@npm%3A4.2.4#builtin<compat/typescript>::version=4.2.4&hash=a45b0e"
bin:
tsc: bin/tsc
tsserver: bin/tsserver
checksum: 3be44317593182e8ce494c114e7ad9b0bb2553a22f3085cc6da4f0a36912c20850daa9be4c898af2ab6fc8b12f430c1c9e46ac715721867cd38643f2350d3ef9
languageName: node
linkType: hard

"yarn-v2@workspace:.":
version: 0.0.0-use.local
resolution: "yarn-v2@workspace:."
dependencies:
typescript: ^4.2.4
languageName: unknown
linkType: soft
5 changes: 3 additions & 2 deletions packages/server/test/support/helpers/e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,7 @@ const e2e = {
Fixtures.installStubPackage(options.project, options.stubPackage)
}

args = ['index.js'].concat(args)
args = options.args || ['index.js'].concat(args)

let stdout = ''
let stderr = ''
Expand Down Expand Up @@ -716,7 +716,7 @@ const e2e = {

return new Bluebird((resolve, reject) => {
debug('spawning Cypress %o', { args })
const sp = cp.spawn('node', args, {
const sp = cp.spawn(options.command || 'node', args, {
env: _.chain(process.env)
.omit('CYPRESS_DEBUG')
.extend({
Expand Down Expand Up @@ -744,6 +744,7 @@ const e2e = {
...(options.noExit ? { CYPRESS_INTERNAL_FORCE_FILEWATCH: '1' } : {}),
})
.extend(options.processEnv)
.extend(options.spawnOpts)
.value(),
})

Expand Down