moving to strict mode #1848
moving to strict mode #1848
Conversation
…m/damienbod/angular-auth-oidc-client into fabiangosebrink-fix/strict-mode-1
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://nice-hill-002425310-1848.centralus.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://nice-hill-002425310-1848.centralus.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://nice-hill-002425310-1848.centralus.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://nice-hill-002425310-1848.centralus.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://nice-hill-002425310-1848.centralus.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://nice-hill-002425310-1848.centralus.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://nice-hill-002425310-1848.centralus.azurestaticapps.net |
1 similar comment
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://nice-hill-002425310-1848.centralus.azurestaticapps.net |
I got some time later this week, I'll try to review this PR. |
|
I don't think it is possible to review all files. @damienbod will test, I also did run a few test, worked fine so far. |
|
Yea, I'll review some of the files within the library source, and also do a few tests. |
|
That is even more than I can ask for. That will be perfect. Thank you! |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://nice-hill-002425310-1848.centralus.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://nice-hill-002425310-1848.centralus.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://nice-hill-002425310-1848.centralus.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://nice-hill-002425310-1848.centralus.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://nice-hill-002425310-1848.centralus.azurestaticapps.net |
1 similar comment
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://nice-hill-002425310-1848.centralus.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://nice-hill-002425310-1848.centralus.azurestaticapps.net |
There was a problem hiding this comment.
Looks great @FabianGosebrink ! 🏆
It also compiles and works on the project I'm working on.
I do got two questions/comments though:
- I notice that
anyis used here and there, I can pick this up after this PR is merged - I also noticed that some operations where the config is used uses the following pattern
tokenRefreshInSeconds ?? 0(I assume this is for if the config option isnullorundefined). Is this correct, because the docs/JSDocs mention a default value. For both cases can something be said, I take the assumption that for these cases we expect the user to configure the option with thenullvalue?
projects/angular-auth-oidc-client/src/lib/callback/interval.service.ts
Outdated
Show resolved
Hide resolved
…rvice.ts Co-authored-by: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com>
That would be nice, yes. Happy to help.
Ah, we should use the default value of 0. However, what if the user specifies/overwrites it with undefined or null? Well, we could let it crash then, because the value is not supported. Thoughts? |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://nice-hill-002425310-1848.centralus.azurestaticapps.net |
I believe this can be added in the future with a new major release. |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://nice-hill-002425310-1848.centralus.azurestaticapps.net |
|
I think we can merge then, can't we? |
No description provided.