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 support for Built-in control flow syntax (#26) #27

Conversation

pmpak
Copy link

@pmpak pmpak commented Nov 20, 2023

BREAKING CHANGE

  • minimum angular version required bumped to 17
  • minimum node version required bumped to v18.13.0 to be aligned with the Angular 17 requirements
  • minimum TypeScript version required bumped to v5.2 to be aligned with the Angular 17 requirements

@pmpak
Copy link
Author

pmpak commented Nov 20, 2023

Closes #26

Because I needed to use imports that don't exist in Angular <= 16 I had to bump up the minimum required version for Angular, Node and TypeScript which leads to a major update in case we proceed on merging it.
In case this is undesirable I could approach it differently but this could lead to a more verbose and fragile workarounds.

@michaelbromley how would you prefer to proceed?

@michaelbromley
Copy link
Member

michaelbromley commented Nov 21, 2023

Wow amazing! Thank you!
Since this change only becomes relevant in the case of a major change to an Angular codebase, I think it makes sense that we make this a major bump for this lib too.

One small thing: the test fail in CI because the workflow file specifies node v16.x Could you update this as part of this PR to at least 18.x (you could also add v20.x so we are covering that version too).

Oh also let's add a row to the compatibility table for v9.x / Angular v17

)

BREAKING CHANGE
- minimum angular version required bumped to 17
- minimum node version required bumped to v18.13.0 to be aligned with the Angular 17 requirements
- minimum TypeScript version required bumped to v5.2 to be aligned with the Angular 17 requirements
@pmpak pmpak force-pushed the feature/add-support-for-built-in-flow-syntax branch from 15e6403 to 9e03b11 Compare November 21, 2023 13:28
@pmpak
Copy link
Author

pmpak commented Nov 21, 2023

oups, the workflow file slipped my attention. I updated now the node versions and the versions in the compatibility table in the README.md

@michaelbromley
Copy link
Member

@michaelbromley michaelbromley merged commit 5cfe5dd into vendure-ecommerce:master Nov 21, 2023
2 checks passed
@michaelbromley
Copy link
Member

Great! I'll do a release shortly 👍

@pmpak
Copy link
Author

pmpak commented Nov 21, 2023

Thank you very much! In the meantime I will make some more tests in a project and fingers crossed there are no strange edge cases.

@pmpak pmpak deleted the feature/add-support-for-built-in-flow-syntax branch November 22, 2023 17:41
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.

2 participants