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: #22038 support esm import for windows #22042

Merged
merged 9 commits into from
Jun 2, 2022
Merged
7 changes: 7 additions & 0 deletions packages/launchpad/cypress/e2e/scaffold-project.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ function scaffoldAndOpenE2EProject (opts: {
cy.contains('E2E Testing').click()
cy.contains('We added the following files to your project:')
cy.contains('Continue').click()
// Going through the loading of config
cy.get('[data-cy="loading-spinner"]')
cy.get('[data-cy="loading-spinner"]').should('not.exist')
// No errrors were encountered
cy.get('[data-testid="error-header"]').should('not.exist')
// Asserts that we've made it through the flow
cy.contains('Choose a Browser')
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 weren't confirming the flow completed, which meant we were missing a failure here

}

function scaffoldAndOpenCTProject (opts: {
Expand Down
17 changes: 5 additions & 12 deletions packages/server/lib/plugins/child/run_require_async_child.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
require('graceful-fs').gracefulify(require('fs'))
const stripAnsi = require('strip-ansi')
const debug = require('debug')(`cypress:lifecycle:child:run_require_async_child:${process.pid}`)
const { pathToFileURL } = require('url')
const tsNodeUtil = require('./ts_node')
const util = require('../util')
const { RunPlugins } = require('./run_plugins')
Expand Down Expand Up @@ -97,12 +98,10 @@ function run (ipc, file, projectRoot) {
// 3a. Yes: Use bundleRequire
// 3b. No: Continue through to `await import(configFile)`
// 4. Use node's dynamic import to import the configFile
let originalError

try {
return require(file)
} catch (err) {
originalError = err
if (!err.stack.includes('[ERR_REQUIRE_ESM]') && !err.stack.includes('SyntaxError: Cannot use import statement outside a module')) {
throw err
}
Expand All @@ -124,16 +123,10 @@ function run (ipc, file, projectRoot) {
debug(`User doesn't have esbuild. Going to use native node imports.`)

// We cannot replace the initial `require` with `await import` because
// Certain modules cannot be dynamically imported. If this throws, however, we want
// to show the original error that was thrown, because that's ultimately the source of the problem
try {
return await import(file)
} catch (e) {
// If we aren't able to import the file at all, throw the original error, since that has more accurate information
// of what failed to begin with
debug('esbuild fallback for loading config failed, throwing original error. node import error: %o', e)
throw originalError
}
// Certain modules cannot be dynamically imported.

// pathToFileURL for windows interop: https://github.com/nodejs/node/issues/31710
return await import(pathToFileURL(file).href)
Copy link
Contributor

Choose a reason for hiding this comment

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

If the file is a .ts file, we will get a problem, though, since native imports do not support .ts.
#22071
I will look into it.

}

throw err
Expand Down
3 changes: 0 additions & 3 deletions system-tests/__snapshots__/config_modules_spec.ts.js

This file was deleted.

7 changes: 0 additions & 7 deletions system-tests/test/config_modules_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,6 @@ describe('cypress config with esm and cjs', function () {
spec: 'app.cy.js',
browser: 'chrome',
expectedExitCode: 1,
snapshot: true,
onStdout (stdout) {
expect(stdout).to.include('nearest parent package.json contains "type": "module" which defines all .ts files in that package scope as ES modules')

// Need to make this stable b/c of filepaths, and snapshot: true is needed to invoke onStdout
return 'STDOUT_ERROR_VALIDATED'
},
})
})

Expand Down