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

build: update project to Angular 17 #2185

Closed
wants to merge 3 commits into from

Conversation

michaelfaith
Copy link
Contributor

Summary

This change updates root dependencies to use Angular 17. It also introduces a new example project with a fully standalone v17 app for testing.

Note: updating to the latest prettier / eslint resulted in some minor formatting updates.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

The esm and esm-isolated tests under the new example 17 project were timing out for me, but I wasn't sure if that was just something wrong with my local environment, since I've not run this project locally before. It would be good to confirm that these are running successfully. If not, I may need some guidance on what might be the issue. Might be related to this issue #2138

This change updates root dependencies to use Angular 17.  It
also introduces a new example project with a fully
standalone v17 app for testing.

Note: updating to the latest prettier / eslint resulted in some minor
formatting updates.
@michaelfaith
Copy link
Contributor Author

michaelfaith commented Nov 13, 2023

@ahnpnl Curious how you'd like to deal with this kind of code. If we upgrade TS in the root to 5.2, several props used in this code don't exist anymore. Would you prefer to keep the root version of TS lower to support this compatibility layer, or take it out and increase min version support?

image

@ahnpnl
Copy link
Collaborator

ahnpnl commented Nov 13, 2023

@ahnpnl Curious how you'd like to deal with this kind of code. If we upgrade TS in the root to 5.2, several props used in this code don't exist anymore. Would you prefer to keep the root version of TS lower to support this compatibility layer, or take it out and increase min version support?

image

The files in src/transformers and src/ngtsc are copy version of Angular codes (with a bit modifications, mostly in replace-resource.ts), which are downloaded via https://github.com/thymikee/jest-preset-angular/blob/main/scripts/prebuild.js.

According to Angular support policy, after 22th Nov 2023, the minimum supported version is v15. That means the files in The files in src/transformers and src/ngtsc need to be able to run with Angular 15, 16 and 17.

If some codes have been removed in v17, but still exists in v15 and v16, we need to:

  • First download the changes in v17
  • Compare with downloaded version of v15 and v16
  • Make the changes in such a way to support all.

@michaelfaith
Copy link
Contributor Author

Great. thanks for the background. I'll look at the new versions of these files

@johncrim
Copy link
Contributor

Thank you for your work on this. I haven't grabbed this branch, but I would bet money that the tests are timing out due to the issue described in #2138. I have not been able to upgrade to Typescript 5 due to #2138. And ng 17 requires TS 5, so that issue definitely needs a fix for ng 17 to work.

@michaelfaith
Copy link
Contributor Author

Yeah, I didn't anticipate the TypeScript compatibility work that's ultimately required to land this Angular upgrade. Realistically, it'll be at least another couple weeks before I can get back on this and start to work through that refactor. Since the TypeScript upgrade could be done separately, if anyone else wants to jump in and do that as a separate change, please feel free.

@ahnpnl
Copy link
Collaborator

ahnpnl commented Jan 3, 2024

Continue in #2198

@michaelfaith
Copy link
Contributor Author

Thanks for wrapping this up. Apologies for the delay on my end. We had a long holiday.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants