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

chore: revert "fix: support ESM projects using TypeScript with ts-node/esm" #22225

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 0 additions & 20 deletions packages/data-context/src/actions/MigrationActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@ import {
} from '../sources/migration'
import { makeCoreData } from '../data'
import { LegacyPluginsIpc } from '../data/LegacyPluginsIpc'
import { hasTypeScriptInstalled } from '../util'

const tsNode = require.resolve('@packages/server/lib/plugins/child/register_ts_node')

export function getConfigWithDefaults (legacyConfig: any) {
const newConfig = _.cloneDeep(legacyConfig)
Expand Down Expand Up @@ -91,23 +88,6 @@ export async function processConfigViaLegacyPlugins (projectRoot: string, legacy
const configProcessArgs = ['--projectRoot', projectRoot, '--file', cwd]
const CHILD_PROCESS_FILE_PATH = require.resolve('@packages/server/lib/plugins/child/require_async_child')

// use ts-node if they've got typescript installed
// this matches the 9.x behavior, which is what we want for
// processing legacy pluginsFile (we never supported `"type": "module") in 9.x.
if (hasTypeScriptInstalled(projectRoot)) {
const tsNodeLoader = `--require ${tsNode}`

if (!childOptions.env) {
childOptions.env = {}
}

if (childOptions.env.NODE_OPTIONS) {
childOptions.env.NODE_OPTIONS += ` ${tsNodeLoader}`
} else {
childOptions.env.NODE_OPTIONS = tsNodeLoader
}
}

const childProcess = fork(CHILD_PROCESS_FILE_PATH, configProcessArgs, childOptions)
const ipc = new LegacyPluginsIpc(childProcess)

Expand Down
59 changes: 1 addition & 58 deletions packages/data-context/src/data/ProjectConfigIpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,17 @@ import { CypressError, getError } from '@packages/errors'
import type { FullConfig, TestingType } from '@packages/types'
import { ChildProcess, fork, ForkOptions } from 'child_process'
import EventEmitter from 'events'
import fs from 'fs-extra'
import path from 'path'
import inspector from 'inspector'
import debugLib from 'debug'
import { autoBindDebug, hasTypeScriptInstalled } from '../util'
import { autoBindDebug } from '../util'
import _ from 'lodash'

const pkg = require('@packages/root')
const debug = debugLib(`cypress:lifecycle:ProjectConfigIpc`)

const CHILD_PROCESS_FILE_PATH = require.resolve('@packages/server/lib/plugins/child/require_async_child')

const tsNodeEsm = require.resolve('ts-node/esm/transpile-only')
const tsNode = require.resolve('@packages/server/lib/plugins/child/register_ts_node')

export type IpcHandler = (ipc: ProjectConfigIpc) => void

export interface SetupNodeEventsReply {
Expand Down Expand Up @@ -242,59 +238,6 @@ export class ProjectConfigIpc extends EventEmitter {

debug('fork child process %o', { CHILD_PROCESS_FILE_PATH, configProcessArgs, childOptions: _.omit(childOptions, 'env') })

let isProjectUsingESModules = false

try {
const pkgJson = fs.readJsonSync(path.join(this.projectRoot, 'package.json'))

isProjectUsingESModules = pkgJson.type === 'module'
} catch (e) {
// project does not have `package.json` or it was not found
// reasonable to assume not using es modules
}

if (!childOptions.env) {
childOptions.env = {}
}

// If they've got TypeScript installed, we can use
// ts-node for CommonJS
// ts-node/esm for ESM
if (hasTypeScriptInstalled(this.projectRoot)) {
if (isProjectUsingESModules) {
// Use the ts-node/esm loader so they can use TypeScript with `"type": "module".
// The loader API is experimental and will change.
// The same can be said for the other alternative, esbuild, so this is the
// best option that leverages the existing modules we bundle in the binary.
// @see ts-node esm loader https://typestrong.org/ts-node/docs/usage/#node-flags-and-other-tools
// @see Node.js Loader API https://nodejs.org/api/esm.html#customizing-esm-specifier-resolution-algorithm
const tsNodeEsmLoader = `--experimental-specifier-resolution=node --loader ${tsNodeEsm}`

if (childOptions.env.NODE_OPTIONS) {
childOptions.env.NODE_OPTIONS += ` ${tsNodeEsmLoader}`
} else {
childOptions.env.NODE_OPTIONS = tsNodeEsmLoader
}
} else {
// Not using ES Modules (via "type": "module"),
// so we just register the standard ts-node module
// to handle TypeScript that is compiled to CommonJS.
// We do NOT use the `--loader` flag because we have some additional
// custom logic for ts-node when used with CommonJS that needs to be evaluated
// so we need to load and evaluate the hook first using the `--require` module API.
const tsNodeLoader = `--require ${tsNode}`

if (childOptions.env.NODE_OPTIONS) {
childOptions.env.NODE_OPTIONS += ` ${tsNodeLoader}`
} else {
childOptions.env.NODE_OPTIONS = tsNodeLoader
}
}
} else {
// Just use Node's built-in ESM support.
// TODO: Consider using userland `esbuild` with Node's --loader API to handle ESM.
}

const proc = fork(CHILD_PROCESS_FILE_PATH, configProcessArgs, childOptions)

return proc
Expand Down
15 changes: 4 additions & 11 deletions packages/data-context/src/data/ProjectConfigManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -260,17 +260,10 @@ export class ProjectConfigManager {
this._eventsIpc.cleanupIpc()
}

this._eventsIpc = new ProjectConfigIpc(
this.options.ctx.nodePath,
this.options.projectRoot,
this.configFilePath,
this.options.configFile,
(cypressError: CypressError, title?: string | undefined) => {
this._state = 'errored'
this.options.ctx.onError(cypressError, title)
},
this.options.ctx.onWarning,
)
this._eventsIpc = new ProjectConfigIpc(this.options.ctx.nodePath, this.options.projectRoot, this.configFilePath, this.options.configFile, (cypressError: CypressError, title?: string | undefined) => {
this._state = 'errored'
this.options.ctx.onError(cypressError, title)
}, this.options.ctx.onWarning)

this._loadConfigPromise = this._eventsIpc.loadConfig()
}
Expand Down
9 changes: 0 additions & 9 deletions packages/data-context/src/util/hasTypescript.ts

This file was deleted.

1 change: 0 additions & 1 deletion packages/data-context/src/util/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,5 @@ export * from './autoBindDebug'
export * from './cached'
export * from './config-file-updater'
export * from './file'
export * from './hasTypescript'
export * from './pluginHandlers'
export * from './urqlCacheKeys'
18 changes: 0 additions & 18 deletions packages/data-context/test/unit/util/hasTypescript.spec.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ describe('Launchpad: Error System Tests', () => {
cy.contains('h1', cy.i18n.launchpadErrors.generic.configErrorTitle)
cy.percySnapshot()

cy.get('[data-testid="error-code-frame"]').should('contain', 'cypress.config.ts:6:10')
cy.get('[data-testid="error-code-frame"]').should('contain', 'cypress.config.ts:6:9')
})
})

Expand Down
2 changes: 1 addition & 1 deletion packages/launchpad/cypress/e2e/error-handling.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ describe('Error handling', () => {
cy.contains('Choose a Browser').should('not.exist')

cy.withCtx((ctx) => {
ctx.actions.file.writeFileInProject('cypress.config.ts', `
ctx.actions.file.writeFileInProject('cypress.config.js', `
import { defineConfig } from 'cypress'
import { defineConfig as viteConfig } from 'vite'
export default defineConfig({
Expand Down
16 changes: 0 additions & 16 deletions packages/server/lib/plugins/child/register_ts_node.js

This file was deleted.

45 changes: 34 additions & 11 deletions packages/server/lib/plugins/child/run_require_async_child.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
require('graceful-fs').gracefulify(require('fs'))
const stripAnsi = require('strip-ansi')
const debugLib = require('debug')
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')

const debug = debugLib(`cypress:lifecycle:child:run_require_async_child:${process.pid}`)
let tsRegistered = false

/**
* Executes and returns the passed `file` (usually `configFile`) file in the ipc `loadConfig` event
Expand All @@ -21,6 +22,14 @@ function run (ipc, file, projectRoot) {
throw new Error('Unexpected: projectRoot should be a string')
}

if (!tsRegistered) {
debug('register typescript for required file')
tsNodeUtil.register(projectRoot, file)

// ensure typescript is only registered once
tsRegistered = true
}

process.on('uncaughtException', (err) => {
debug('uncaught exception:', util.serializeError(err))
ipc.send('childProcess:unhandledError', util.serializeError(err))
Expand Down Expand Up @@ -83,9 +92,14 @@ function run (ipc, file, projectRoot) {
// Config file loading of modules is tested within
// system-tests/projects/config-cjs-and-esm/*
const loadFile = async (file) => {
try {
debug('Loading file %s', file)
// 1. Try loading the configFile
// 2. Catch the "ERR_REQUIRE_ESM" error
// 3. Check if esbuild is installed
// 3a. Yes: Use bundleRequire
// 3b. No: Continue through to `await import(configFile)`
// 4. Use node's dynamic import to import the configFile

try {
return require(file)
} catch (err) {
if (!err.stack.includes('[ERR_REQUIRE_ESM]') && !err.stack.includes('SyntaxError: Cannot use import statement outside a module')) {
Expand All @@ -96,16 +110,25 @@ function run (ipc, file, projectRoot) {
debug('User is loading an ESM config file')

try {
// We cannot replace the initial `require` with `await import` because
// Certain modules cannot be dynamically imported.
// pathToFileURL for windows interop: https://github.com/nodejs/node/issues/31710
const fileURL = pathToFileURL(file).href
debug('Trying to use esbuild to run their config file.')
// We prefer doing this because it supports TypeScript files
require.resolve('esbuild', { paths: [process.cwd()] })

debug(`importing esm file %s`, fileURL)
debug(`They have esbuild, so we'll load the configFile via bundleRequire`)
const { bundleRequire } = require('bundle-require')

return await import(fileURL)
return (await bundleRequire({ filepath: file })).mod
} catch (err) {
debug('error loading file via native Node.js module loader %s', err.message)
if (err.stack.includes(`Cannot find module 'esbuild'`)) {
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.

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

throw err
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
require('../../../spec_helper')

const tsNodeUtil = require('../../../../lib/plugins/child/ts_node')
const runRequireAsyncChild = require('../../../../lib/plugins/child/run_require_async_child')
const resolve = require('../../../../lib/util/resolve')

describe('lib/plugins/child/run_require_async_child', () => {
beforeEach(function () {
Expand All @@ -15,6 +17,28 @@ describe('lib/plugins/child/run_require_async_child', () => {
mockery.deregisterMock('@cypress/webpack-batteries-included-preprocessor')
})

describe('typescript registration', () => {
beforeEach(() => {
sinon.stub(tsNodeUtil, 'register')
sinon.stub(resolve, 'typescript').returns('/path/to/typescript.js')
})

it('registers ts-node only once when typescript module found', function () {
runRequireAsyncChild(this.ipc, 'cypress.config.js', 'proj-root')
runRequireAsyncChild(this.ipc, 'cypress.config.js', 'proj-root')

expect(tsNodeUtil.register).to.be.calledWith(
'proj-root',
'cypress.config.js',
)

expect(tsNodeUtil.register).to.be.calledOnce
})

// FIXME: need to validate that TS is checked once when ts is not found as well
it.skip('checks for typescript only once if typescript module was not found')
})

describe('errors', () => {
beforeEach(function () {
sinon.stub(process, 'on')
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { numberOne } from '../../src/foo'

it('works', () => {
const number = 1

expect(numberOne).to.equal(number)
})

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
it('works', () => {
expect(true).to.be.true
})
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
{
"type": "module",
"projectFixtureDirectory": "simple_passing"
"type": "module"
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { numberOne } from '../../src/foo'

it('works', () => {
const number = 1

expect(numberOne).to.equal(number)
})

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const numberOne = 1