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: update to ng18 #158

Closed
wants to merge 1 commit into from
Closed

feat: update to ng18 #158

wants to merge 1 commit into from

Conversation

Erbenos
Copy link

@Erbenos Erbenos commented Oct 5, 2024

PR Checklist

  • The commit message follows our guidelines: CONTRIBUTING.md#commit
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[x] Other... Please describe: Update Dependencies

What is the new behavior?

Does this PR introduce a breaking change?

[X] Yes
[ ] No

Now depends on ~6 months old major version of Angular instead of ~12 months old one. No changes in code were necessary, all e2e tests still pass. Really only bumped dependencies.

Basically I want to update my own application thats using this library to Angular 18, but given this is a dependency of mine
that up to this day depends on Angular ^17, it needs to be updated first. I noticed that #157 exists after I already did the change, and this unlike that one does not combine both unneeded behavior changes with dependency updates. Which should hopefully make it easier to approve given the #157 is yet to be merged nearly a quarter after it was opened.

Copy link

stackblitz bot commented Oct 5, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@Erbenos
Copy link
Author

Erbenos commented Oct 5, 2024

I am moving it to draft cause I have noticed that the "Custom Template" part of playground does not correctly update its visibility state. This isn't caught by e2e tests.

I also didn't notice the nested package.json.

@Erbenos
Copy link
Author

Erbenos commented Oct 5, 2024

I updated the nested package.json.

I am moving it from draft because the visibility state issue mentioned in #158 (comment) happens even when I try the playground when on top of master (with npm ci) so its not related. Not clear why it behaves correctly in https://ngneat.github.io/helipopper/ though.

@Erbenos Erbenos marked this pull request as ready for review October 5, 2024 06:28
"tippy.js": "6.3.7"
},
"peerDependencies": {
"@angular/core": ">=17.0.0"
Copy link
Member

Choose a reason for hiding this comment

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

Any breaking changes that we need to change the min version?

@Erbenos
Copy link
Author

Erbenos commented Oct 6, 2024

Ughhh, I am stupid, nevermind. I was only looking at the root package.json not realizing the published package will get the nested one and the nested one specifies Angular >=17.0.0, you can in fact install it with Angular 18 just fine. My bad. Closing this.

@Erbenos Erbenos closed this Oct 6, 2024
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