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

fix(angular): ng add @ionic/angular in standalone projects #28523

Merged
merged 10 commits into from
Nov 20, 2023
Merged

Conversation

sean-perkins
Copy link
Contributor

@sean-perkins sean-perkins commented Nov 14, 2023

Issue number: Resolves #28514


What is the current behavior?

When using the @ionic/angular schematic in an Angular 17 project (ng add @ionic/angular), developers will receive an error preventing the schematic from running.

Additionally, the previous implementations of the schematic are out of sync with the current state of the Ionic starters:

  • variables.css is empty and missing Ionic's defaults
  • ionic.config.json is not created
  • Schematic does not have support for module vs. standalone projects.

What is the new behavior?

  • ng add @ionic/angular works with Angular 17 projects
  • ng add @ionic/angular has fallback behavior for Angular 16 projects using AppModule
  • Schematics now includes the proper variables.css from Ionic starters
  • Ionicons assets will no longer be copied when being added to a standalone project
  • Refactors a majority of the implementation to use the utilities that come directly from @angular-devkit/schematics and @schematics/angular.
  • Sets the @ionic/angular-toolkit CLI configuration and schematics configuration in the angular.json
  • Creates missing ionic.config.json

Does this introduce a breaking change?

  • Yes
  • No

Other information

Dev-build: 7.5.5-dev.11700239837.1925bbdb

To test this PR:

  1. Install Angular CLI v17 - npm install -g @angular/cli@17
  2. Create a new project - ng new angular-17
  3. Use the dev-build: - ng add @ionic/[email protected]
  4. Confirm the prompts
  5. Validate that provideIonicAngular({}) is added to the app.config.ts
  6. Validate that ionic.config.json was created
  7. Validate that angular.json was updated with the @ionic/angular-devkit configurations

Now verify legacy behavior:

  1. Install Angular CLI v16 - npm install -g @angular/cli@16
  2. Create a new project - ng new angular-16
  3. Use the dev-build - ng add @ionic/[email protected]
  4. Confirm the prompts
  5. Validate that IonicModule.forRoot({}) is added to the app.module.ts
  6. Validate the ionicons glob pattern is added to the angular.json
  7. Validate the ionic.config.json was created
  8. Validate the angular.json was updated with the @ionic/angular-devkit configurations

@github-actions github-actions bot added the package: angular @ionic/angular package label Nov 14, 2023
@sean-perkins sean-perkins marked this pull request as ready for review November 14, 2023 00:23
@sean-perkins

This comment was marked as resolved.

@sean-perkins
Copy link
Contributor Author

Let me know if you have any questions on how to review. I can run through creating a new angular project, testing the schematic and looking at the diffs 👍

packages/angular/src/schematics/utils/config.ts Outdated Show resolved Hide resolved
packages/angular/src/schematics/utils/config.ts Outdated Show resolved Hide resolved
packages/angular/src/schematics/utils/config.ts Outdated Show resolved Hide resolved
@@ -61,8 +61,8 @@
"zone.js": ">=0.11.0"
},
"devDependencies": {
"@angular-devkit/core": "^14.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Unsure if this is related, but I get the following error when I try to run ionic serve:

liamdebeasi@MacBook-Pro angular-17 % ionic serve
> ng run app:serve --host=localhost --port=8100
[ng] Error: Unknown arguments: host, port

[ERROR] ng has unexpectedly closed (exit code 1).

This happens in ng16 and ng17.

Copy link
Contributor Author

@sean-perkins sean-perkins Nov 16, 2023

Choose a reason for hiding this comment

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

I don't believe the schematic updates should impact that. This schematic's execution context is purely for the ng add command. After that it has no usage.

For example if I use the dev-build to run ng add and then install the latest version of @ionic/angular I get the same exception when using ionic serve:

sean@MacBook-Air ~/d/i/i/a/angular-blank (main)> ionic serve
> ng run app:serve --host=localhost --port=8100
[ng] Error: Unknown arguments: host, port

Developers really should be using the start command script with their projects: npm start.

Edit: But we should probably track this problem for the Ionic CLI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok looked into this more. You cannot assume to use ionic serve in a new Angular project. If the project name is not "app" it won't work.

For example my project is called angular-blank, so the command would need to be:

ng run angular-blank:serve --host=localhost --port=8100
> ng run angular-blank:serve --host=localhost --port=8100


Initial Chunk Files | Names         |  Raw Size
polyfills.js        | polyfills     |  82.71 kB | 
styles.css          | styles        |  32.69 kB | 
main.js             | main          |  23.47 kB | 

                    | Initial Total | 138.87 kB

Application bundle generation complete. [2.358 seconds]
Watch mode enabled. Watching for file changes...
  ➜  Local:   http://localhost:8100/

Our schematic could change the project name, but I think that is extremely high risk to break a lot of things in the consumers project.

Copy link
Contributor

Choose a reason for hiding this comment

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

If this problem happened before the dev build them I'm fine keeping it out of scope. I assumed you could use ionic serve because the ionic.config.json file was present, but maybe that's not the case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like you can use the Ionic CLI if you provide the project name for your angular project. If a name is not provided it defaults to app.

e.g.:

ionic serve --project=angular-blank

packages/angular/src/schematics/add/index.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@liamdebeasi liamdebeasi left a comment

Choose a reason for hiding this comment

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

Good to go once my final comment is addressed.

packages/angular/src/schematics/utils/config.ts Outdated Show resolved Hide resolved
@sean-perkins sean-perkins added this pull request to the merge queue Nov 20, 2023
Merged via the queue into main with commit c07312e Nov 20, 2023
46 checks passed
@sean-perkins sean-perkins deleted the sp/FW-5585 branch November 20, 2023 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: angular @ionic/angular package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: ng add does not work with standalone angular projects
2 participants