fix(launchpad): support default export #20383
Conversation
|
Test summary
Run details
View run in Cypress Dashboard ➡️ FlakinessThis 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 |
lmiller1990
changed the base branch from
unify-1073-config-migration
to
10.0-release
| @@ -7,7 +7,7 @@ module.exports = defineConfig({ | |||
| // We've imported your old cypress plugins here. | |||
| // You may want to clean this up later by importing these. | |||
| setupNodeEvents(on, config) { | |||
| return require('./')(on, config) | |||
There was a problem hiding this comment.
These were all wrong, how the heck did this get merged? This is why I don't generally like snapshots; they don't test behavior.
lmiller1990
changed the title
| } | ||
|
|
||
| const pluginFile = fs.readFileSync(path.join(createConfigOptions.projectRoot, pluginPath), 'utf8') | ||
| const hasExportDefault = pluginFile.includes('export default') |
There was a problem hiding this comment.
This would be more foolproof I believe
| const hasExportDefault = pluginFile.includes('export default') | |
| const hasExportDefault = /\nexport default /.test(pluginsFile) |
We could also create a harmonizing function inside of cypress to use instead of require.
import { compatibleRequire } from 'cypress'
TBH, even the use of the require function at all seems like a bad idea.
An import at the top of the file would probably work better.
There was a problem hiding this comment.
One of the nice things with require is if there are any side effects, it won't execute until setupNodeEvents is called - that's closer to the original behavior. I think someone suggested a top-level import in the Jira ticket but this makes for a more minimal code change, which is nice. Why is require a bad idea? Any edge case I'm missing?
There was a problem hiding this comment.
Also couldn't you have
export default (on, config) {}And your regexp would not work?
There was a problem hiding this comment.
For the regexp, I am afraid of people using "task" to scaffold files before they start testing.
This could really throw you off.
My all-time fav is the babel way:
export default defineConfig({
e2e: setupNodeEvent(on, config){
const pluginsFunction = require('./cypress/plugins')
return pluginsFunction.default ? pluginsFunction.default(on, config) : pluginsFunction(on, config)
}
})There was a problem hiding this comment.
I don't think we want to add a conditional to the generated config; it seems weird that we can't just give them the correct code out of the box.
There was a problem hiding this comment.
I also don't want to actually import their file to see if it has a .default property since that would trigger any side effects during config generation, not really ideal.
There was a problem hiding this comment.
Ok, I ended up implementing this with babel.
lmiller1990
dismissed stale reviews from davidmunechika and elevatebart
via
96f6eda
|
Fixed merge conflict, gonna merge this up since it had two ✅ |
* 10.0-release: fix: comment link to accurate docs (#20437) fix: scaffold commands file (#20398) fix(launchpad): support default export (#20383) feat(launchpad): support for Vue CLI 5 (#20413) fix: UNIFY-676 browsers should be configurable in setupNodeEvents (#20367) fix: make launchpad link open default browser (#20399) fix(icons): publish the files in the package fix: build icons in build-prod (#20411) test: migrate module-api to 10.0 chore: build this branch test: migrate module_api to system tests (#20265) chore: remove pkg/driver //@ts-nocheck final (#20169) chore: fix "cannot find module" in clone-repo-and-checkout-release-branch (#20293) chore: Update Chrome (beta) to 99.0.4844.45 (#20234) chore: fix CI cache state for darwin (#20339) Add TODO comments feedback chore: move tests to its own file.
Closes: https://cypress-io.atlassian.net/browse/UNIFY-1073
Adds support for migrating
cypress/plugins/index.tstocypress.config.tswhere the pluginsFile hasexport default.Demo (Cypress internal team only).