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: Fix issues with Cypress.require() and TypeScript #25931

Merged
merged 39 commits into from
Mar 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
5a39d85
fix: make typescript a dependency instead of a devDependency [run ci]
chrisbreiding Feb 23, 2023
6f7ddaa
Merge remote-tracking branch 'origin/develop' into issue-25885-cypres…
chrisbreiding Feb 24, 2023
bda413b
chore: updating v8 snapshot cache
Feb 24, 2023
ab5e6ea
chore: updating v8 snapshot cache
Feb 24, 2023
10a58d5
chore: updating v8 snapshot cache
Feb 24, 2023
974ff30
Merge branch 'issue-25885-cypress-require-typescript' of github.com:c…
chrisbreiding Feb 27, 2023
a61a752
Merge remote-tracking branch 'origin/develop' into issue-25885-cypres…
chrisbreiding Feb 27, 2023
7d77b28
revert dep change
chrisbreiding Feb 27, 2023
43ba9db
fix: process origin callback in child process with correct typescript…
chrisbreiding Feb 28, 2023
638aee9
Merge remote-tracking branch 'origin/develop' into issue-25885-cypres…
chrisbreiding Feb 28, 2023
f931fb9
run ci
chrisbreiding Feb 28, 2023
ceb0805
add branch back to workflows [run ci]
chrisbreiding Feb 28, 2023
4eb3c4b
return undefined instead of null [run ci]
chrisbreiding Feb 28, 2023
08df049
fix [run ci]
chrisbreiding Feb 28, 2023
452de15
uhhh send the project root [run ci]
chrisbreiding Feb 28, 2023
4dee523
add branch to artifact persistence [run ci]
chrisbreiding Feb 28, 2023
b162c6c
delete moved file
chrisbreiding Mar 1, 2023
e15bf7c
unskip and fix run_plugins spec
chrisbreiding Mar 2, 2023
6495ecf
add binary system test [run ci]
chrisbreiding Mar 2, 2023
11ceab7
Merge remote-tracking branch 'origin/develop' into issue-25885-cypres…
chrisbreiding Mar 2, 2023
cf608a6
add changelog entry
chrisbreiding Mar 2, 2023
62f0d19
add yarn.lock [run ci]
chrisbreiding Mar 2, 2023
f3a1771
add date [run ci]
chrisbreiding Mar 2, 2023
9d825d7
remove failed attempt at a system test
chrisbreiding Mar 3, 2023
ee4e142
add unit tests to batteries included preprocessor [run ci]
chrisbreiding Mar 3, 2023
2bd68c9
Merge remote-tracking branch 'origin/develop' into issue-25885-cypres…
chrisbreiding Mar 3, 2023
bf4ef2f
Update workflows.yml
chrisbreiding Mar 3, 2023
749c388
chore: updating v8 snapshot cache
Mar 3, 2023
442aa3c
chore: updating v8 snapshot cache
Mar 3, 2023
21c9ca3
chore: updating v8 snapshot cache
Mar 3, 2023
fa10505
refactor getFullWebpackOptions
chrisbreiding Mar 3, 2023
826b2a3
Merge branch 'issue-25885-cypress-require-typescript' of github.com:c…
chrisbreiding Mar 3, 2023
1fc6816
Merge remote-tracking branch 'origin/develop' into issue-25885-cypres…
chrisbreiding Mar 3, 2023
51c897c
Merge branch 'develop' into issue-25885-cypress-require-typescript
chrisbreiding Mar 7, 2023
8d33891
revert snapshot meta changes
chrisbreiding Mar 7, 2023
b8ac30d
no fancy syntax :P
chrisbreiding Mar 7, 2023
5399345
one more
chrisbreiding Mar 7, 2023
1618d98
see if this fixes integration spec timeout
chrisbreiding Mar 8, 2023
af35e90
fix test
chrisbreiding Mar 8, 2023
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
4 changes: 4 additions & 0 deletions cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ _Released 03/14/2023 (PENDING)_
- It is now possible to control the number of connection attempts to the browser using the CYPRESS_CONNECT_RETRY_THRESHOLD Environment Variable. Learn more [here](https://docs.cypress.io/guides/references/advanced-installation#Environment-variables). Addressed in [#25848](https://github.com/cypress-io/cypress/pull/25848).
- The Debug page is now able to show real-time results from in-progress runs. Addresses [#25759](https://github.com/cypress-io/cypress/issues/25759).

**Bugfixes:**

- Fixed an issue where using `Cypress.require()` would throw the error `Cannot find module 'typescript'`. Fixes [#25885](https://github.com/cypress-io/cypress/issues/25885).
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this fixing a regression or did it never work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It worked with regular require, but hasn't worked with Cypress.require() since it was re-released.


**Misc:**

- Removed "New" badge in the navigation bar for the debug page icon. Addresses [#25925](https://github.com/cypress-io/cypress/issues/25925)
Expand Down
20 changes: 11 additions & 9 deletions npm/webpack-batteries-included-preprocessor/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,16 @@ const hasTsLoader = (rules) => {

const addTypeScriptConfig = (file, options) => {
// shortcut if we know we've already added typescript support
if (options.__typescriptSupportAdded) return
if (options.__typescriptSupportAdded) return options

const webpackOptions = options.webpackOptions
const rules = webpackOptions.module && webpackOptions.module.rules

// if there are no rules defined or it's not an array, we can't add to them
if (!rules || !Array.isArray(rules)) return
if (!rules || !Array.isArray(rules)) return options

// if we find ts-loader configured, don't add it again
if (hasTsLoader(rules)) return
if (hasTsLoader(rules)) return options

const TsconfigPathsPlugin = require('tsconfig-paths-webpack-plugin')
// node will try to load a projects tsconfig.json instead of the node
Expand Down Expand Up @@ -57,6 +57,8 @@ const addTypeScriptConfig = (file, options) => {
})]

options.__typescriptSupportAdded = true

return options
}

/**
Expand Down Expand Up @@ -163,7 +165,7 @@ const preprocessor = (options = {}) => {
options.webpackOptions = options.webpackOptions || getDefaultWebpackOptions()

if (options.typescript) {
addTypeScriptConfig(file, options)
options = addTypeScriptConfig(file, options)
}

if (process.versions.pnp) {
Expand All @@ -181,13 +183,13 @@ preprocessor.defaultOptions = {
}

preprocessor.getFullWebpackOptions = (filePath, typescript) => {
const options = { typescript }

options.webpackOptions = getDefaultWebpackOptions()
const webpackOptions = getDefaultWebpackOptions()

addTypeScriptConfig({ filePath }, options)
if (typescript) {
return addTypeScriptConfig({ filePath }, { typescript, webpackOptions }).webpackOptions
}

return options.webpackOptions
return webpackOptions
}

// for testing purposes, but do not add this to the typescript interface
Expand Down
2 changes: 1 addition & 1 deletion npm/webpack-batteries-included-preprocessor/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"description": "Cypress preprocessor for bundling JavaScript via webpack with dependencies included and support for various ES features, TypeScript, and CoffeeScript",
"private": false,
"scripts": {
"test": "mocha test/e2e/*.spec.* --timeout 4000",
"test": "mocha test/**/*.spec.* --timeout 4000",
"lint": "eslint --ext .js,.ts,.json, ."
},
"dependencies": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const runAndEval = async (fileName, options) => {
eval(contents.toString())
}

describe('features', () => {
describe('webpack-batteries-included-preprocessor features', () => {
beforeEach(async () => {
preprocessor.__reset()

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
const { expect } = require('chai')

const preprocessor = require('../../index')

describe('webpack-batteries-included-preprocessor', () => {
context('#getFullWebpackOptions', () => {
it('returns default webpack options (and does not add typescript config if no path specified)', () => {
const result = preprocessor.getFullWebpackOptions()

expect(result.node.global).to.be.true
expect(result.module.rules).to.have.length(3)
expect(result.resolve.extensions).to.eql(['.js', '.json', '.jsx', '.mjs', '.coffee'])
})

it('adds typescript config if path is specified', () => {
const result = preprocessor.getFullWebpackOptions('file/path', 'typescript/path')

expect(result.module.rules).to.have.length(4)
expect(result.module.rules[3].use[0].loader).to.include('ts-loader')
})
})
})
6 changes: 3 additions & 3 deletions packages/app/cypress/e2e/settings.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,8 @@ describe('App: Settings', () => {
cy.visitApp()
cy.get(SidebarSettingsLinkSelector).click()
cy.findByText('Project settings').click()
cy.get('[data-cy="file-match-indicator"]').contains('41 matches')
cy.get('[data-cy="spec-pattern"]').contains('tests/**/*')
cy.get('[data-cy="file-match-indicator"]').contains('19 matches')
cy.get('[data-cy="spec-pattern"]').contains('tests/**/*.(js|ts|coffee)')
})

it('shows the Experiments section', () => {
Expand Down Expand Up @@ -237,7 +237,7 @@ describe('App: Settings', () => {

cy.get('[data-cy="config-code"]').within(() => {
cy.get('[data-cy-config="config"]').contains('tests/_fixtures')
cy.get('[data-cy-config="config"]').contains('tests/**/*')
cy.get('[data-cy-config="config"]').contains('tests/**/*.(js|ts|coffee)')
cy.get('[data-cy-config="config"]').contains('tests/_support/spec_helper.js')
cy.get('[data-cy-config="env"]').contains('REMOTE_DEBUGGING_PORT')
cy.get('[data-cy-config="env"]').contains('INTERNAL_E2E_TESTING_SELF')
Expand Down
7 changes: 6 additions & 1 deletion packages/driver/src/cross-origin/origin_fn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,13 @@ const getCallbackFn = async (fn: string, file?: string) => {
// in the outer scope (see the return value below), assign the function to it
// in the inner scope, then call the function with the args
const callbackName = '__cypressCallback'

const response = await fetch('/__cypress/process-origin-callback', {
body: JSON.stringify({ file, fn: `${callbackName} = ${fn};` }),
body: JSON.stringify({
file,
fn: `${callbackName} = ${fn};`,
projectRoot: Cypress.config('projectRoot'),
}),
headers: {
'Content-Type': 'application/json',
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,19 @@
import { getFullWebpackOptions } from '@cypress/webpack-batteries-included-preprocessor'
import md5 from 'md5'
import { fs } from 'memfs'
import * as path from 'path'
import webpack from 'webpack'

const md5 = require('md5')
Copy link
Contributor Author

@chrisbreiding chrisbreiding Mar 1, 2023

Choose a reason for hiding this comment

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

Moved this file into the child process. The main reason for the content changes are converting it from typescript to vanilla JS, since we currently don't have the processing set up for TS within the child process.

const { fs } = require('memfs')
const path = require('path')
const webpack = require('webpack')
const VirtualModulesPlugin = require('webpack-virtual-modules')

interface Options {
file: string
fn: string
}
const resolve = require('../../util/resolve')

// @ts-expect-error - webpack expects `fs.join` to exist for some reason
fs.join = path.join

export const processCallback = ({ file, fn }: Options) => {
const processCallback = ({ file, fn, projectRoot }) => {
const { getFullWebpackOptions } = require('@cypress/webpack-batteries-included-preprocessor')

const source = fn.replace(/Cypress\.require/g, 'require')
const webpackOptions = getFullWebpackOptions(file, require.resolve('typescript'))
const typescriptPath = resolve.typescript(projectRoot)
const webpackOptions = getFullWebpackOptions(file, typescriptPath)
Comment on lines +15 to +16
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the root of the fix. Instead of resolving typescript with our own, we get the path to it the same way we do for regular file preprocessing by finding the user's installed typescript.


const inputFileName = md5(source)
const inputDir = path.dirname(file)
Expand Down Expand Up @@ -45,17 +42,16 @@ export const processCallback = ({ file, fn }: Options) => {

const compiler = webpack(modifiedWebpackOptions)

// @ts-expect-error
compiler.outputFileSystem = fs

return new Promise<string>((resolve, reject) => {
const handle = (err: Error) => {
return new Promise((resolve, reject) => {
const handle = (err) => {
if (err) {
return reject(err)
}

// Using an in-memory file system, so the usual restrictions on sync
// methods don't apply, since this won't throw an EMFILE error
// this won't throw an EMFILE error since it's using an in-memory file
// system, so the usual restrictions on sync methods don't apply
// eslint-disable-next-line no-restricted-syntax
const result = fs.readFileSync(outputPath).toString()

Expand All @@ -65,3 +61,7 @@ export const processCallback = ({ file, fn }: Options) => {
compiler.run(handle)
})
}

module.exports = {
processCallback,
}
70 changes: 38 additions & 32 deletions packages/server/lib/plugins/child/run_plugins.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const resolve = require('../../util/resolve')
const browserLaunch = require('./browser_launch')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most of the changes in here was just moving a couple methods around so that the order made a little more sense (to me at least) when I was updating the tests for this. The only thing I added were the lines referencing _process:cross:origin:callback.

const util = require('../util')
const validateEvent = require('./validate_event')
const crossOrigin = require('./cross_origin')

const UNDEFINED_SERIALIZED = '__cypress_undefined__'

Expand All @@ -37,23 +38,25 @@ class RunPlugins {
this.registeredEventsByName = {}
}

invoke = (eventId, args = []) => {
const event = this.registeredEventsById[eventId]

return event.handler(...args)
}

getDefaultPreprocessor (config) {
const tsPath = resolve.typescript(config.projectRoot)
const options = {
...tsPath && { typescript: tsPath },
/**
* This is the only publicly-used method of this class
*
* @param {Object} config
* @param {Function} setupNodeEventsFn
*/
runSetupNodeEvents (config, setupNodeEventsFn) {
debug('project root:', this.projectRoot)
if (!this.projectRoot) {
throw new Error('Unexpected: projectRoot should be a string')
}

debug('creating webpack preprocessor with options %o', options)
debug('passing config %o', config)

const webpackPreprocessor = require('@cypress/webpack-batteries-included-preprocessor')
this.ipc.on('execute:plugins', (event, ids, args) => {
this.execute(event, ids, args)
})

return webpackPreprocessor(options)
return this.load(config, setupNodeEventsFn)
}

load (initialConfig, setupNodeEvents) {
Expand Down Expand Up @@ -110,8 +113,9 @@ class RunPlugins {
// events used for parent/child communication
registerChildEvent('_get:task:body', () => {})
registerChildEvent('_get:task:keys', () => {})
registerChildEvent('_process:cross:origin:callback', crossOrigin.processCallback)

Promise
return Promise
.try(() => {
debug('Calling setupNodeEvents')

Expand All @@ -120,7 +124,7 @@ class RunPlugins {
.tap(() => {
if (!this.registeredEventsByName['file:preprocessor']) {
debug('register default preprocessor')
registerChildEvent('file:preprocessor', this.getDefaultPreprocessor(initialConfig))
registerChildEvent('file:preprocessor', this._getDefaultPreprocessor(initialConfig))
}
})
.then((modifiedCfg) => {
Expand Down Expand Up @@ -156,6 +160,7 @@ class RunPlugins {
case 'after:run':
case 'after:spec':
case 'after:screenshot':
case '_process:cross:origin:callback':
return util.wrapChildPromise(this.ipc, this.invoke, ids, args)
case 'task':
return this.taskExecute(ids, args)
Expand All @@ -172,6 +177,12 @@ class RunPlugins {
}
}

invoke = (eventId, args = []) => {
const event = this.registeredEventsById[eventId]

return event.handler(...args)
}

wrapChildPromise (invoke, ids, args = []) {
return Promise.try(() => {
return invoke(ids.eventId, args)
Expand All @@ -191,9 +202,9 @@ class RunPlugins {

taskGetBody (ids, args) {
const [event] = args
const taskEvent = _.find(this.registeredEventsById, { event: 'task' }).handler
const taskEvent = _.find(this.registeredEventsById, { event: 'task' })
const invoke = () => {
const fn = taskEvent[event]
const fn = taskEvent && taskEvent.handler[event]

return _.isFunction(fn) ? fn.toString() : ''
}
Expand All @@ -202,8 +213,8 @@ class RunPlugins {
}

taskGetKeys (ids) {
const taskEvent = _.find(this.registeredEventsById, { event: 'task' }).handler
const invoke = () => _.keys(taskEvent)
const taskEvent = _.find(this.registeredEventsById, { event: 'task' })
const invoke = () => _.keys(taskEvent ? taskEvent.handler : {})

util.wrapChildPromise(this.ipc, invoke, ids)
}
Expand Down Expand Up @@ -241,22 +252,17 @@ class RunPlugins {
util.wrapChildPromise(this.ipc, invoke, ids, [arg])
}

/**
*
* @param {Function} setupNodeEventsFn
*/
runSetupNodeEvents (config, setupNodeEventsFn) {
debug('project root:', this.projectRoot)
if (!this.projectRoot) {
throw new Error('Unexpected: projectRoot should be a string')
_getDefaultPreprocessor (config) {
const tsPath = resolve.typescript(config.projectRoot)
const options = {
...tsPath && { typescript: tsPath },
}

debug('passing config %o', config)
this.load(config, setupNodeEventsFn)
debug('creating webpack preprocessor with options %o', options)

this.ipc.on('execute:plugins', (event, ids, args) => {
this.execute(event, ids, args)
})
const webpackPreprocessor = require('@cypress/webpack-batteries-included-preprocessor')

return webpackPreprocessor(options)
}
}

Expand Down
1 change: 1 addition & 0 deletions packages/server/lib/plugins/child/validate_event.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const isObject = (event, handler) => {
const eventValidators = {
'_get:task:body': isFunction,
'_get:task:keys': isFunction,
'_process:cross:origin:callback': isFunction,
'after:run': isFunction,
'after:screenshot': isFunction,
'after:spec': isFunction,
Expand Down
6 changes: 3 additions & 3 deletions packages/server/lib/routes-e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import reporter from './controllers/reporter'
import client from './controllers/client'
import files from './controllers/files'
import type { InitializeRoutes } from './routes'
import { processCallback } from './cross-origin/process-callback'
import * as plugins from './plugins'

const debug = Debug('cypress:server:routes-e2e')

Expand Down Expand Up @@ -54,11 +54,11 @@ export const createRoutesE2E = ({

routesE2E.post(`/${config.namespace}/process-origin-callback`, bodyParser.json(), async (req, res) => {
try {
const { file, fn } = req.body
const { file, fn, projectRoot } = req.body

debug('process origin callback: %s', fn)

const contents = await processCallback({ file, fn })
const contents = await plugins.execute('_process:cross:origin:callback', { file, fn, projectRoot })

res.json({ contents })
} catch (err) {
Expand Down
Loading