-
Notifications
You must be signed in to change notification settings - Fork 3.3k
feat: Target modern ES with default preprocessor #15274
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
Conversation
Thanks for taking the time to open a PR!
|
Test summaryRun details
View run in Cypress Dashboard ➡️ Flakiness
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
@@ -433,7 +433,7 @@ describe('runner/cypress retries.ui.spec', { viewportWidth: 600, viewportHeight: | |||
it('throws when set via this.retries in test', () => { |
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.
We eval these suite functions, and since they're no longer compiled down to 'suite 1': function () {}
, e.g., evaling results in a syntax error if they're not explicitly function expressions.
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.
looks good 👍
@@ -84,6 +84,7 @@ export const getCommonConfig = () => { | |||
[require.resolve('@babel/plugin-proposal-class-properties'), { loose: true }], | |||
], | |||
presets: [ | |||
// the chrome version should be synced with npm/webpack-batteries-included-preprocessor/index.js |
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.
...and npm/webpack-batteries-included-preprocessor/index.js should probably be synced with
cypress/packages/server/lib/browsers/chrome.ts
Lines 507 to 510 in 6e70281
await criClient.ensureMinimumProtocolVersion('1.3') | |
.catch((err) => { | |
throw new Error(`Cypress requires at least Chrome 64.\n\nDetails:\n${err.message}`) | |
}) |
64 or 63?
is the minimum possible chrome version documented in a central location anywhere? i guess we can't publicly claim that we support chrome 63/64 though, since we only test on the latest chrome version AFAIK
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.
Interesting, I didn't know about this. Makes sense to bump the other ones to 64 then.
I don't think this is documented at all and we don't have an official minimum version of Chrome. All we can say is we definitely don't support Chrome < 64, but yeah, can't guaranteed Chrome 64+ will work.
@@ -33,7 +33,8 @@ const getDefaultWebpackOptions = (file, options = {}) => { | |||
}], | |||
], | |||
presets: [ | |||
[require.resolve('@babel/preset-env'), { modules: 'commonjs' }], | |||
// the chrome version should be synced with packages/web-config/webpack.config.base.ts |
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.
see other file for comment
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
User facing changelog
Additional details
We now target Chrome 63 for spec code, which is the same as the driver code.
How has the user experience changed?
The generated spec code will look more similar to the source code. This should make the dashboard experience better, in particular, since it displays the generated code for failures.
PR Tasks
cypress-documentation
?type definitions
?cypress.schema.json
?