-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
feat: vue-cli and nuxt preset for CT object API architecture #20956
Conversation
Thanks for taking the time to open a PR!
|
Test summaryRun details
View run in Cypress Dashboard ➡️ 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 |
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.
Should we add coverage for vuecli/nuxt typescript projects?
Some comments, nothing blocking. Tested and it works great 👍
|
||
// Add to this list to focus on a particular permutation | ||
// TODO: run vuecli4-vue2 tests once cypress/vue-2 is bundled | ||
const ONLY_PROJECTS: ProjectDirs[number][] = ['vuecli5-vue3'] |
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.
I think we should have a test for vuecli4-vue3
as well.
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.
export function vueCliHandler ({ devServerConfig }: PresetHandler): Configuration { | ||
try { | ||
const config = require.resolve('@vue/cli-service/webpack.config', { | ||
paths: [devServerConfig.cypressConfig.projectRoot], |
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.
The sourceRelativeWebpackModules.ts
has the framework and importPath (though since this is coming from @vue/cli-service
it would have to be mapped) which can be used to do a direct require. This works as well, what are your thoughts on which system to use? I'm fine with either but going this route would make the work done inside of sourceRelativeWebpackModules.ts
for frameworks largely redundant.
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.
I think using require.resolve.paths
is nice since it gives better stack traces if the module isn't found: #20956 (review)
My understanding is the sourceRelativeWebpackModules
is more for sourcing the modules we ship in the binary and pointing to the right ones (eg, webpack-dev-server
, html-webpack-plugin
, etc).
@@ -49,8 +78,8 @@ export function makeWebpackConfig ( | |||
config: CreateFinalWebpackConfig, | |||
) { | |||
const { module: webpack } = config.sourceWebpackModulesResult.webpack | |||
const userWebpackConfig = config.devServerConfig.webpackConfig as Partial<Configuration> | |||
const frameworkWebpackConfig = config.frameworkConfig as Partial<Configuration> | |||
const userWebpackConfig = modifyWebpackConfigForCypress(config.devServerConfig.webpackConfig as Partial<Configuration>) |
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.
Do we have to do this twice or can we just modify the mergedConfig? I did something similar here:
const finalUserWebpackConfig = merge(frameworkWebpackConfig ?? {}, userWebpackConfig ?? {}) |
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.
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.
Looking good, the require.resolve.paths
is nice b/c it shows all the places it searched for the module.
|
||
const { getWebpackConfig } = require(nuxt) | ||
|
||
const webpackConfig = await getWebpackConfig() |
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.
😢 that this forces async everywhere. Was looking into nuxt internal to understand why it does, and it seems only to be because of the ability to have nuxt options be async: https://github.com/nuxt/nuxt.js/blame/db11f7bf937cdbb96ff0067bdee1aa153978e4e0/packages/config/src/load.js#L63
Oh well, probably not worth tapping into internal APIs, but I was half wondering if we could just source from @nuxt/webpack directly.
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.
Yes I shed a tear when I saw this was async. We can def explore tapping into the internals, but it seems better to use their public API, at least for now, where the goal is stability. Can definitely dig into this more later, or even make a PR to nuxt (we are good friends with them).
…o-vue-cli fix: cleanup for #20956
User facing changelog
Add out-of-the-box support for Vue CLI and Nuxt.js in the new Object API CT architecture.
Now you easily configure Vue CLI with Cypress CT, like this:
Additional details
How has the user experience changed?
Heavily inspired by @ZachJW34's branch: https://github.com/cypress-io/cypress/pull/20954/files#diff-c96246e7228f1e151b9a664f2465af2aee6f1eefef8cc69b2cd1b7e4d650a0f0
Since we did not bundle
cypress/vue-2
with the binary, I skipped that test. It can be added once https://cypress-io.atlassian.net/browse/UNIFY-1253 is completed.PR Tasks
cypress-documentation
?type definitions
?cypress.schema.json
?