Skip to content
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: Add Angular CT Schematic #24374

Merged
merged 35 commits into from
Jan 24, 2023
Merged

Conversation

w0wka91
Copy link
Contributor

@w0wka91 w0wka91 commented Oct 25, 2022

User facing changelog

Additional details

Why was this change necessary? What is affected by this change?

This change enables the user to generate a new component including a component spec file. By implementing this we increase users adoption of Angular CT and improve over DX by doing all the boilerplate work for developers.
This change also adds @cypress/schematic to the default schematics. This enables the user to use all of the commands without prefixing them with the package name.

Steps to test

Generate a new Angular Application and add @cypress/schematic with ng add <path/to/cypress/schematic>

How has the user experience changed?

Before the user had to execute the following commands to generate a component and a component spec file:

ng generate component button
ng generate @cypress/schematic:spec button --component

Now the user can just execute this command:
ng generate component button
Also, the user can now use any @cypress/schematic generate command without prefixing it with the package name:
So this ng generate @cypress/schematic:spec {name}
becomes this: ng generate spec {name}

PR Tasks

  • [ X] Have tests been added/updated?
  • [ X] Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
  • [ X] Has a PR for user-facing changes been opened in cypress-documentation?
  • [ X] Have API changes been updated in the type definitions?

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Oct 25, 2022

Thanks for taking the time to open a PR!

@w0wka91 w0wka91 changed the title Issue 22976 feat: Add Angular CT Schematic Oct 25, 2022
@ZachJW34 ZachJW34 requested review from admah and jordanpowell88 and removed request for marktnoonan and amehta265 October 25, 2022 14:05
Copy link
Contributor

@jordanpowell88 jordanpowell88 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR looks great! I would love to see a cypress tests that validates all of this though here.

@lmiller1990 lmiller1990 self-requested a review November 23, 2022 00:01
Copy link
Contributor

@lmiller1990 lmiller1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The latest changes seem to be breaking CI, I tried to fix it but I'm actually not clear on what's supposed to be generated - my understanding is a cy.ts file with cy.mount, but this is not happening.

I tried to debug this, it looks like it points to ng-component/generators/index.ts that has:

 externalSchematic('@schematics/angular', 'component', {
        ...options,
        skipTests: true,
      }),

I have rebased and resolved the conflicts w/ develop.

Also tested, not working for me?

$ ng generate component baz
CREATE src/app/baz/baz.component.scss (0 bytes)
CREATE src/app/baz/baz.component.html (18 bytes)
CREATE src/app/baz/baz.component.spec.ts (578 bytes)
CREATE src/app/baz/baz.component.ts (264 bytes)
UPDATE src/app/app.module.ts (524 bytes)

No cy.ts is generated.

schematicCollections: ['@cypress/schematic', ...angularJsonVal?.cli?.schematicCollections ?? []],
}

if (cli.schematicCollections.indexOf('@schematics/angular') === -1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

schematicCollections is only for Angular-14 apps. Previously, the value was defaultCollection but it isn't an array. Should we override the default value instead for Angular 13 apps?

@lmiller1990
Copy link
Contributor

Looks like we've got a real CI failure.

@ZachJW34 ZachJW34 self-requested a review December 8, 2022 18:08
@ZachJW34 ZachJW34 assigned ZachJW34 and unassigned BlueWinds Dec 8, 2022
@emilyrohrbough emilyrohrbough added the npm: @cypress/schematic @cypress/schematic package issues label Dec 8, 2022
@ZachJW34
Copy link
Contributor

@w0wka91 I think it makes sense to not change the defaultCollection as a user might have a value here and changing it could break their schematic invocations. Pushing to the schematicCollections is fine since it supports multiple values. WDYT?

@w0wka91
Copy link
Contributor Author

w0wka91 commented Jan 20, 2023

@ZachJW34 Makes sense. So I would suggest we append the defaultCollection (if it it not null) to the schematicCollections array. What do you think?

@ZachJW34
Copy link
Contributor

@w0wka91 I think we drop support for Angular 13 for this version. I don't think schematics should support multiple ranges. I tried implementing what I suggested and any schematics invoked via ng g @cypress/schematic would not work since this package has dependencies on Angular 14. When executing the schematic, the package would resolve v14 of angular inside the context of an Angular 13 app.

@jordanpowell88 is it cool to drop support for v13 for this schematic? We could support it but we'll have to manage our deps differently.

@ZachJW34
Copy link
Contributor

@w0wka91 I chatted with @jordanpowell88 a bit and pushed up a commit that removes support for v13. The component schematic was not working with v13 due to misconfigured dependencies, and furthermore the component schematic adheres to the v14 of the angular component schematic (standalone) so it wouldn't work regardless of the deps issue. Hoping for a green build!

@ZachJW34 ZachJW34 requested a review from lmiller1990 January 23, 2023 21:00
@ZachJW34
Copy link
Contributor

@lmiller1990 Can you re-review? If it looks good and we merge, we should include a BREAKING CHANGE in the footer of the commit message since we are dropping support for Angular v13 for this schematic package.

@lmiller1990
Copy link
Contributor

lmiller1990 commented Jan 24, 2023

Sure thing, main question: it looks like Angular 13 still has a couple hundred k installs / week: https://www.npmjs.com/package/@angular/core?activeTab=versions. It is two majors behind now, though, and my understanding is Angular culture is to upgrade to the latest versions pretty quickly.

Are we okay with breaking the schematic in a Cypress minor? I am not sure if we've got an official cadence around this. Or is there some way users on Angular 13 could still use this older version?

@ZachJW34
Copy link
Contributor

ZachJW34 commented Jan 24, 2023

Ideally I think the @cypress/schematic should shadow the release of Angular. Users could still install older versions of the schematic that would correspond to their Angular version. This schematic package isn't included in the binary/cli so we can have breaking changes to the package without affecting the cypress app itself.

With the addition of the ng g @cypress/schematic:component schematic, the schema (options) for the schematic rely on the Angular component schematic since the cypress component schematic calls through to the Angular schematic. When the contract changes (which is the case for the v13 to v14 upgrade introducing the standalone option), this package needs to update to accommodate. @jordanpowell88 thoughts?

The best user experience is to have the cypress component schematic match 1:1 with the Angular schematic which we can only do by dropping support for older versions if the contract changes. We could support multiple versions but we would lose this experience, specifically when running ng g @cypress/schematic:component --help as it would show only a subset of the available options due to use tailoring the inputs to conform across multiple interfaces.

@lmiller1990
Copy link
Contributor

Sounds good. Let's merge this up. A release went out just now, sow this will be in the next release.

@lmiller1990 lmiller1990 merged commit af6be6f into cypress-io:develop Jan 24, 2023
tgriesser added a commit that referenced this pull request Jan 26, 2023
* develop: (27 commits)
  refactor: remove unused cloud routes (#25584)
  chore: fix issue template formatting issue (#25587)
  fix: fixed issue with CT + electron + run mode not exiting properly (#25585)
  chore(deps): update dependency ua-parser-js to v0.7.33 [security] (#25561)
  fix: add alternative binary names for edge-beta (#25456)
  chore: add batch execution to CloudDataSource (#22457)
  chore: End a/b campaigns for aci smart banners (#25504)
  chore: release @cypress/schematic-v2.5.0
  fix(cypress-schematic): do not disable e2e support file (#25400)
  chore: adding memory issue template (#25559)
  feat: Add Angular CT Schematic (#24374)
  chore: enforce changelog entries on PR reviews (#25459)
  chore: bump package.json to 12.4.0 [run ci] (#25556)
  feat: Add 'type' option to `.as` to store aliases by value (#25251)
  chore: release @cypress/webpack-dev-server-v3.2.3
  feat: Display line break in cy.log (#25467)
  chore: update types (#25538)
  fix: Revert "fix: adding emergency garbage collection for chromium-based browsers" (#25546)
  fix: percy - wait to take snapshot until previous tooltips are gone (#25522)
  feat: support data-qa selector in selector playground (#25475)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CT Issue related to component testing npm: @cypress/schematic @cypress/schematic package issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Angular CT Schematic