-
Notifications
You must be signed in to change notification settings - Fork 689
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
Angular 9 upgrade #717
Comments
Added a PR ☝️ . Tested by running the |
I am attempting to upgrade my module wrapper that uses this package to angular 9 and I am facing an error that didn't exists in angular 8.
This is specific to server side flow. There were some significant changes to how the server is set up to be more friendly to non web based architecture. I think this package would need some attention for angular 9 :) |
@thebaron24 Did you try that using my branch? |
I will give that a try. Is your branch fully server side rendering? |
I did use a fresh angular 9 generated app with these commands npx -p @angular/cli ng new angular-universal cd angular-universal npx -p @angular/cli ng add @nguniversal/express-engine |
I'm not sure what you mean by:
The branch I mention for PR #718 doesn't do anything SSR specific, it just upgrades the library project to Angular 9. That's also what this issue is about. I'm now a bit confused what your earlier comment was talking about? I had assumed you had your own Angular 8 project with version 8.x of this library that had SSR working, and you wanted to upgrade your app and this library to versions 9.x. If you have another issue, different from just technically upgrading the library in this repo to Angular 9, it might be good to open up a new issue, provide a minimal repro and so forth, if possible? Edit: apologies, above might've come off wrongly. I'm happy to help, either here or in a fresh issue, but at least for me personally some clarification is needed. Let us know how the community can help out 👍 |
No problem @jeroenheijmans, I see your comments on this package section regularly and know you are looking to help. Let me try and clarify my comment. I noticed your previous remark that updating angular in a sample application seemed to go over fine. And that may be true for the client side implementation of this package and build paths. However, I just spent two weeks setting up an Angular 8 module that uses this package to do the oauth using the code flow. I created a fresh angular 9 generated application and dropped my oauth module into it, and imported the server side module and browser module into the fresh app and it threw the above error when I tried to run it. I know this was written with server side flows (aks universal angular) in mind but I don't think the latest version works with it. I was just showing you an error I found. |
I'm still confused a bit. Are your comments feedback on how we upgrade(d) this library to Angular 9? Are you proposing additional changes to the upgrade in my PR? Or does my PR break something that used to work? It might be good to open up a fresh issue for things related to your wrapper library and Angular Universal? |
@jeroenheijmans I think my comment was premature. I will wait until this package officially releases an angular 9 version and do my upgrade and testing of my module wrapper. If I find and issue then, I will open a separate issue. Apologies for the confusion. I appreciate the quickness in which you guys are doing the upgrade. |
Any info on when the support for version 9 will be released ? |
Let's upgrade to Angular 9 in preparation for a new release of the library, shall we?
The text was updated successfully, but these errors were encountered: