Skip to content

Conversation

@alisaduncan
Copy link

@alisaduncan alisaduncan commented Sep 16, 2025

PR Checklist

Please check if your PR fulfills the following requirements:

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

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Adding Tests
  • Build related changes
  • CI related changes
  • Documentation changes
  • Other... Please describe:
    Bump min version of Angular supported

What is the current behavior?

Doesn't support Angular v17 - v19

Issue Number: N/A

What is the new behavior?

Support Angular v17 - v19

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Reviewers

@@ -0,0 +1,47 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like there are few rules from the old files missing here

Copy link
Author

Choose a reason for hiding this comment

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

the import rule is handled implicitly now (i tested with adding a rando import).
Jest is added to the spec file config

I haven't checked the node one though, but not sure it is needed (which node scripts are we worried about?) also the plugin hasn't been updated since 2020... maybe this is for e2e???

"auth",
"oauth2"
],
"exports": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this removed?

Copy link
Contributor

@jaredperreault-okta jaredperreault-okta Sep 16, 2025

Choose a reason for hiding this comment

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

Have you looked into merging this package.json with the top-level one?

Copy link
Author

Choose a reason for hiding this comment

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

it's handled with the new version of Angular. I double checked the package.json in the dist dir

(there's also a warning in the console if you leave it)

Copy link
Author

Choose a reason for hiding this comment

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

sorry, just saw your second comment.

i'm not sure i understand the question. this is the package.json we distribute, so i'd think we wouldn't want to combine it with the root project level one. (i bet APF wouldn't handle that merging well anyway)

Copy link
Contributor

@jaredperreault-okta jaredperreault-okta Sep 17, 2025

Choose a reason for hiding this comment

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

Right now we have

package.json
lib/package.json
lib/src/..

Ideally I'd like to combine these into a single package.json and src dir. But if that is not possible that's fine (aka get rid of the lib dir)

Copy link
Author

Choose a reason for hiding this comment

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

yeah, that's not possible as ng-packgr expects a sibling package.json to include in the lib.

the dir doesnt have to be named lib though, but it will need to be organized into a directory

@alisaduncan alisaduncan marked this pull request as ready for review October 22, 2025 21:56
@edunham
Copy link

edunham commented Oct 22, 2025

opening this PR and finding out how much change the version bump required is like

giphy

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