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

chore: support ivy based partial compilation for @ngqp/core #225

Conversation

pumano
Copy link

@pumano pumano commented Nov 4, 2022

Library should be compiled using "compilationMode": "partial" for not using ngcc
Also default target for 14 angular is es2020
Update browserlist to actual due to bugs with old data
Change legacy "fullTemplateTypeCheck" to actual "strictTemplates"

BREAKING CHANGE: due to partial ivy compilation it's not supported by angular before 11.1 so bump version to 15

closes #224

This pull request relates to or closes the following issues:

I have verified that…

  • the commits in this pull request follow the conventionalcommits pattern
  • tests have been updated or added
  • documentation has been updated to reflect the changes made

Copy link
Collaborator

@Airblader Airblader left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, I left a couple comments / questions. Please also make sure to sign your commit(s) for the DCO.

package.json Outdated
"@angular/platform-browser-dynamic": "~11.0.7",
"@angular/router": "~11.0.7",
"@angular/service-worker": "~11.0.7",
"@angular/animations": "~14.0.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

This also updates ngqp-demo. What about the issues with Webpack? For me this doesn't run locally.

Copy link
Author

Choose a reason for hiding this comment

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

That is because you use raw-loader for load ts files as string for examples. That raw-loader is removed from webpack 5 and angular cli 14 uses webpack 5

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm well aware of the problem, but I don't understand the change. If this simply doesn't work we of course can't merge it.

@@ -1,6 +1,6 @@
{
"name": "@ngqp/core",
"version": "14.1.0",
"version": "15.0.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please undo this change, this is done during release. Also, move the

BREAKING CHANGE: due to partial ivy compilation it's not supported by angular before 11.1 so bump version to 15

comment into the commit message, because that is where conventional commits need it. But leave out the "version 15" there, that is decided during release. Just an explanation of why it is breaking: the dependencies bump. 🙂

closes TNG#224

Revert "chore: support ivy based partial compilation for @ngqp/core"

This reverts commit 979f038.

chore: fix
@pumano pumano force-pushed the feature/angular-14-partial-compilation-support branch from 979f038 to e6fc605 Compare November 6, 2022 10:21
@pumano
Copy link
Author

pumano commented Nov 6, 2022

@Airblader OK I revert my work, just changed to ivyEnabled to true. I have problems to sign-off, if you can try to change it on your own and publish and decline my PR.

@Airblader
Copy link
Collaborator

Can you please try to get the sign-off to work? The DCO is a company requirement.

The commit message currently also doesn't really make much sense; it talks about reverting a commit.

@pumano pumano closed this Nov 6, 2022
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.

Proper support of angular 14+ (Not working in angular 16 due to absent ngcc)
2 participants