-
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
feat(launchpad): update CT setup and config scaffolding #20893
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 |
@tgriesser I didn't implement that - the ticket says we can go either way. I just went with the "default" workflow, which just shows and confirms all the dependencies, if the user has them installed. The user can then just click continue. I made it more explicit in the tests that use the system-tests infrastructure to install the node modules. See: b487860 Here's how it looks, screenshot from an e2e test: |
@tgriesser @ZachJW34 please re-review, I merged in 10.0-release and responded to all comments. We can merge this last once the other tickets for the CT stuff are done, but still good to review so it's ready to go. |
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.
Went through the deps, this is what I think we should land on for the required deps permutations:
Enumerated options:
| framework | bundler | required deps |
| ---------------- | ------- | ------------------------------- |
| react | webpack | react >= 16, webpack >= 4 |
| create-react-app | webpack | react >= 16, react-scripts >=4 |
| next | webpack | next >= 11, react >= 16 |
| vue | webpack | vue >= 2, webpack >= 4 |
| vue-cli | webpack | vue >= 2, @vue/cli-service >= 4 |
| nuxt | webpack | vue = 2, nuxt = 2 |
| react | vite | react >=16, vite >= 2 |
| vue | vite | vue >= 2, vite >= 2 |
> Note: `typescript` is a required dependency (for all projects) if the user chooses typescript during project setup
My reasoning behind the list is that we should only ask them to install deps we know we need for ct to work. For create-react-app
, all we need is react
and react-scripts
. We don't need webpack
since we can source it from react-scripts
. Same for vue-cli
.
I think we should get all the presets merged and then come to a final decision on this list. I'd like to update the tech-brief with this list as well.
import { devServer } from '@cypress/react/plugins/react-scripts' | ||
} | ||
|
||
return dedent` |
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 cypress.config.js
should also use defineConfig
if it is available.
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.
Did not realise. I will make this change at the end once all the presets are merged, along with updating the dependencies based on whatever decision we arrive at.
|
||
await this.detectLanguage() | ||
debug('detectedLanguage %s', this.data.detectedLanguage) | ||
this.data.chosenLanguage = this.data.detectedLanguage || 'js' | ||
|
||
try { | ||
const detected = detect(await fs.readJson(path.join(this.ctx.currentProject, 'package.json'))) | ||
const detected = detect(this.ctx.currentProject) |
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.
Can we remove the try/catch now that we're not awaiting? Nothing else seems like it should (reasonably) throw.
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.
Done
Ok, Object API seems in a working state - I'll give this one a bit more of a test and get it merged. Edit: still blocked by various things... see this message. |
* fix: small ui tweaks for switch testing type (#20901) * fix: add missing component-index.html (#20926) * move files from data-context to scaffold-config * dynamically modify support file based on selected framework * scaffold correct supportFile for CT based on framework * update scaffolded files Co-authored-by: Zachary Williams <ZachJW34@gmail.com>
* 10.0-release: (25 commits) fix: stop running spec when switching specs (#21038) fix: remove asset size warnings and enable nuxt e2e tests (#21074) feat: swap the #__cy_root id selector to become data-cy-root for component mounting (#20951) fix: Doc changes around vue2 (#21066) feat: Add vue2 package from npm/vue/v2 branch (#21026) skip failing test fix: add possible frameworks to object API config (#21056) fix snapshot spacing fix system test fix snapshot update snapshots rename spec files rename files update snapshots release 9.5.4 [skip ci] fix(regression): cy.pause() should not be ignored with `cypress run --headed --no-exit` (#20877) fix: add missing Cypress.Commands.addAll() types (#20894) chore: Don't store video and screenshot artifacts for runs (#20979) chore: Update Chrome (beta) to 101.0.4951.26 (#20957) chore: remove parallelism from test-binary-against-repo jobs (#21004) ...
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's all coming together!
…e-config * 10.0-release: fix: make error on integration folder point to e2e (#20853) fix(unify): Update Cypress Dashboard Service Link in Login Modal (#21084) chore: fix windows node_modules install step (#21098) fix: webpack integration tests for app w webpack-dev-server-fresh (#21093) fix: move non spec files on migration (#21054) Bumping electron version in root chore(deps): Bumping electron dependency to 15.5.1 (#21072) fix: resolves correctly specPattern (#21057) feat: replace reconfigure button on settings page with link to config doc (#21077) feat(launchpad): update CT setup and config scaffolding (#20893) fix: cy.type('{enter}') on <input> elements submits the form correctly after Firefox 98. (#21042) chore: making the npm deps for vue, react, and vue2 use 0.0.0-dev (#21081) fix(cli): show additional mitigation steps for max path length error (#21047) fix: Plugin error when sending on disconnected IPC channel (#21011) chore: add internal types for cy.session command (#21028) chore: Update Chrome (stable) to 100.0.4896.88 (#21043)
User facing changelog
cypress.config.js
for new project (see images below)webpack-dev-server
,@cypress/react
, etc. We only ask for things liketypescript
,@vue/cli
(if you chosevue-cli
,next
, etc).Additional details
Video demo (shows slight bug in generated
cypress.config.js
, fixed since recording).Do not show specific versions of Vue CLI, react-scripts etc.
Do not show Webpack v4 and v5 - just "webpack"
Dependency Install Screen w/ Invalid Version
Note: there's not such Vue 4, I just hacked
node_modules
to show how it looks if you have a version outside the expected one.Note: it will not ask you to install packages you've already got (note
npm install
command only hasvue
).Note:
npm install vue
instead ofnpm install vue@3
- 3 is the default. If you need a non default version, it'll tell younpm install blah@version
.New
cypress.config.js
using Object API (which is not functioning until #20861 lands)Another example via Percy Diff
https://percy.io/cypress-io/cypress/builds/17057778/changed/958248060?browser=chrome&browser_ids=23&subcategories=unreviewed%2Cchanges_requested&viewLayout=side-by-side&viewMode=new&width=800&widths=800
How has the user experience changed?
PR Tasks
cypress-documentation
?type definitions
?cypress.schema.json
?