Skip to content

Commit

Permalink
Revert "fix: support ESM projects using TypeScript with ts-node/esm (#…
Browse files Browse the repository at this point in the history
…22118)"

This reverts commit abd986a.
  • Loading branch information
ryanthemanuel committed Jun 10, 2022
1 parent a39cbc6 commit 19d6f1b
Show file tree
Hide file tree
Showing 63 changed files with 158 additions and 318 deletions.
20 changes: 0 additions & 20 deletions packages/data-context/src/actions/MigrationActions.ts
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
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
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
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.

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
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
@@ -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
@@ -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
@@ -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.

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

This file was deleted.

@@ -0,0 +1,7 @@
import { numberOne } from '../../src/foo'

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

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

This file was deleted.

@@ -0,0 +1 @@
export const numberOne = 1

0 comments on commit 19d6f1b

Please sign in to comment.