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 provideAuth for standalone applications #1643

Merged
merged 7 commits into from
May 24, 2023

Conversation

timdeschryver
Copy link
Contributor

This PR extracts the config into the _provideAuth method, which in its turn is used in the existing AuthModule and the new provideAuth method (this is the best name that I can think of, feel free to suggest another method name).

The config is also extracted into the auth-config file so it can be reused.

This solution is based on how we did it with the NgRx APIs.

Part of #1599

@damienbod
Copy link
Owner

This looks really great. @FabianGosebrink @timdeschryver Can we merge both of these PRs?

I will use the docs from the the PR to test.

Anything missing?

Thanks for the 2 PRs

@timdeschryver
Copy link
Contributor Author

Happy new year @damienbod and @FabianGosebrink , my best wishes 🎆

To answer #1639 (comment), yes I think we should first merge this PR and then update the other one (sorry for the confusion, but I had some time left for this PR).

I don't know how to make the CI pipeline happy though. To create the standalone provider we're using the makeEnvironmentProviders method from Angular, which is only available in v15 AFAIK. Previous errors were fixed by re-declaring the type/interface on our end, but since this is a method we can't just reimplement it on our side.

@FabianGosebrink
Copy link
Collaborator

Hey @timdeschryver , thanks for your wishes. Happy new year and all the best for you, too.

This would mean we would drop support for A14, right?

Thanks

@timdeschryver timdeschryver force-pushed the feat/provide-auth branch 2 times, most recently from 7a70866 to 59abc50 Compare January 5, 2023 17:48
@timdeschryver
Copy link
Contributor Author

Yea, I can't think of a way to support A14 (see latest commits where I tried some things).
But, for me personally, this PR can wait until we got more breaking changes though, or until more members from the community are asking for this feature.

@FabianGosebrink
Copy link
Collaborator

Hey @timdeschryver , thanks for this PR. I would say that we merge this one once A16 is out. Then we can drop A14 support IMHO. What do you think?

@timdeschryver
Copy link
Contributor Author

Sounds great @FabianGosebrink !
I will check from time to time to rebase this PR .

@damienbod
Copy link
Owner

@timdeschryver Thanks a million for these 2 PRs

@timdeschryver @FabianGosebrink

I think now is the right time to complete these PRs and add this. Good?

Greetings Damien

@timdeschryver
Copy link
Contributor Author

If we drop support for A14, we can also clean up the "this type can be dropped when Angular 14 support" comments.

@timdeschryver
Copy link
Contributor Author

@FabianGosebrink if it's OK for you I can also create a PR to drop support for A14.

@FabianGosebrink
Copy link
Collaborator

Hey @timdeschryver , thank you so much for this. Yes, if you want to, and have time, you can create a PR for this. We should also exclude the pipeline step for V14. I am short on time until end of May and begin of June, then I have more time to help.

@FabianGosebrink FabianGosebrink merged commit 25a9290 into damienbod:main May 24, 2023
@timdeschryver timdeschryver deleted the feat/provide-auth branch May 24, 2023 10: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.

3 participants