-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Changes from 33 commits
5a39d85
6f7ddaa
bda413b
ab5e6ea
10a58d5
974ff30
a61a752
7d77b28
43ba9db
638aee9
f931fb9
ceb0805
4eb3c4b
08df049
452de15
4dee523
b162c6c
e15bf7c
6495ecf
11ceab7
cf608a6
62f0d19
f3a1771
9d825d7
ee4e142
2bd68c9
bf4ef2f
749c388
442aa3c
21c9ca3
fa10505
826b2a3
1fc6816
51c897c
8d33891
b8ac30d
5399345
1618d98
af35e90
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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') | ||
}) | ||
}) | ||
}) |
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') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
|
@@ -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() | ||
|
||
|
@@ -65,3 +61,7 @@ export const processCallback = ({ file, fn }: Options) => { | |
compiler.run(handle) | ||
}) | ||
} | ||
|
||
module.exports = { | ||
processCallback, | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,7 @@ const resolve = require('../../util/resolve') | |
const browserLaunch = require('./browser_launch') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
const util = require('../util') | ||
const validateEvent = require('./validate_event') | ||
const crossOrigin = require('./cross_origin') | ||
|
||
const UNDEFINED_SERIALIZED = '__cypress_undefined__' | ||
|
||
|
@@ -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) { | ||
|
@@ -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') | ||
|
||
|
@@ -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) => { | ||
|
@@ -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) | ||
|
@@ -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) | ||
|
@@ -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?.handler[event] | ||
|
||
return _.isFunction(fn) ? fn.toString() : '' | ||
} | ||
|
@@ -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' })?.handler | ||
const invoke = () => _.keys(taskEvent || {}) | ||
|
||
util.wrapChildPromise(this.ipc, invoke, ids) | ||
} | ||
|
@@ -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) | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 withCypress.require()
since it was re-released.