-
Notifications
You must be signed in to change notification settings - Fork 189
Upgrade to Ivy package format #386
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
Conversation
Hello, the Angular 13 is here... |
Waiting for update |
Hey @arshaw, I don't mean to be pushy about this, just hoping this PR catches your eye ;) Let me know if there is anything I can do to help get this merged faster? |
@Timebutt I'm very sorry for the delay. I'll review/merge/release it in the next week or two. |
Awesome, curious to hear what your thoughts are ;) |
When running
My general understanding is that "View Engine" is deprecated and will eventually be removed and "Ivy" is the new next-level Angular compiler/packager. So I want to encourage you folks to publish an Ivy distribution of More info: |
Hey @nunoarruda, that's exactly what this PR is for ;) The core change to make this happen is this (along with upgrading to Angular 13, that enables this new setting):
|
Hey @Timebutt yesterday I noticed in a project that Angular 13.1 + Wondering if you have any idea if this PR will also fix that. |
Hey @nunoarruda, I'm facing the exact same problem with the latest Angular version. I'm not sure what's causing it yet, but I know this PR doesn't address the problem yet as I tried to use the build artifacts from my PR in my app and it still had the same problem :/ This is what I'm seeing, I'm guessing it's the same for you?
It also doesn't look like it's caused by |
On my side, after trying to navigate to the lazy-loaded route, I get this error: GET http://localhost:4200/src_app_lazy-route_lazy-route_module_ts.js net::ERR_ABORTED 404 (Not Found)
ERROR Error: Uncaught (in promise): ChunkLoadError: Loading chunk src_app_lazy-route_lazy-route_module_ts failed.
(error: http://localhost:4200/src_app_lazy-route_lazy-route_module_ts.js)
ChunkLoadError: Loading chunk src_app_lazy-route_lazy-route_module_ts failed.
(error: http://localhost:4200/src_app_lazy-route_lazy-route_module_ts.js)
at Object.__webpack_require__.f.j (jsonp chunk loading:27)
at ensure chunk:6
at Array.reduce (<anonymous>)
at Function.__webpack_require__.e (ensure chunk:5)
at loadChildren (app-routing.module.ts:4)
at RouterConfigLoader.loadModuleFactory (router.mjs:3977)
at RouterConfigLoader.load (router.mjs:3955)
at router.mjs:3087
at doInnerSub (mergeInternals.js:19)
at outerNext (mergeInternals.js:14)
at resolvePromise (zone.js:1213)
at resolvePromise (zone.js:1167)
at zone.js:1279
at ZoneDelegate.invokeTask (zone.js:406)
at Object.onInvokeTask (core.mjs:25437)
at ZoneDelegate.invokeTask (zone.js:405)
at Zone.runTask (zone.js:178)
at drainMicroTaskQueue (zone.js:582)
at ZoneTask.invokeTask [as invoke] (zone.js:491)
at invokeTask (zone.js:1600) I also noticed on the Lazy Chunk Files | Names | Raw Size
src_app_lazy-route_lazy-route_module_ts.js | lazy-route-lazy-route-module | 5.46 kB | After importing Lazy Chunk Files | Names | Raw Size
src_app_lazy-route_lazy-route_module_ts.js, lazy-route-lazy-route-module.css | lazy-route-lazy-route-module | 595.32 kB | I believe that new/extra css file is the problem, this didn't happen with Angular 13.0. I will try to create a StackBlitz demo with this today and report to the Angular team. Smells like some issue on the Angular 13.1 side-of-things since fullcalendar was working fine with Angular 13.0. |
@nunoarruda there seems to be a fix already here: angular/angular-cli#22363. Not merged yet, but that seems to address the problem so you could use |
Ok. Thanks for your reply. |
FYI the lazy loading issue is fixed on Angular CLI v13.1.2 |
Hey @arshaw, not sure if you have some time available in the near future to review this? I feel it's a pretty straightforward PR that brings a nice improvement for anybody already on Angular 13 🤞 |
I had the same problem, so I commented out all the import 'main.css" in the main.js file of each fullcalendar's component (daygrid, timeline...) i added a script that does in postinstall and i can safely update to angular 13.1.2 To try if it works, remove the .angular folder before running ng serve. No css problem so far after doing this |
@NicolasKritter are these issues you are seeing in this branch of |
Sorry nope it is an issue with the current version, I just jumped into this pull request from an issue that reference this one, In fact it is more related to your finding
As it is not only in daygrid but all comp If you want I can move my comment proposing a dirty quick fix in another issue for the current version |
…l merge from pull request fullcalendar#386
Hi all, I'm trying to merge this for the next release but I am encountering a problem. I can compile the
However, when I try to use it within the Angular 12 example project (one major version down from 13) via npm-link, I get this error:
Is the |
Hmm, it should be given we are using the |
Another hurdle before releasing, I'd like an angular 13 example project. Can someone create one in an |
Is there anything (perhaps small : ) that I could help move this forward @arshaw? I have a customer/project that really needs to move forward and the calendar is the lynch-pin. |
I made a script on monkey-patch the problem (run after npm install
hope it can help you move forward waiting for a solution (note that I use this patch on the released version of fullcalendar, not this branch and with angular 13.1.1 |
Thanks @NicolasKritter, I will see if I can give it a try but ultimately I need to have an updated build from npm. |
can I ask when we can have this pr on the main branch? |
@NicolasKritter, the angular maintainers restored the ability to do .css imports. If you are getting that error you might be using an old version. The main hurdle to merging this PR still stands: the package that it produces is not backward compatible with Angular 12. I am still getting this error when trying to compile from an Angular 12 project. I took the lazy approach and used the existing codebase, and merely bumped the dependencies:
Things work in both angular 12 and 13. I've released this as @fullcalendar/angular version 5.10.2. Please install it and give it a try. Some more work needs to be done on this PR to make it backward compatible with angular 12. I'm leaving it open and hope we can get it merged at some point. Seems like it provides good benefits, like being consumed via Ivy. |
Hey @arshaw, sorry for not getting back earlier but I was hoping to get back to you with some news on backwards compatibility but from my investigation I just don't think it's possible. Any other Angular library I've seen that currently publishes an Ivy package is incompatible with any Angular version prior to 13. What you'll see, is they release a new major version of the package, clearly listing what Angular version is supported by what version of the package (example: ngx-quill). As such, I think releasing it as |
or v13 to keep in sync with angular version? |
@Timebutt in theory it can be compatible with Angular 10+ but depends on Angular version, application engine, and library engine (see https://gist.github.com/LayZeeDK/61caba93df1ec1a0788c94a973c8dfac). Still, I believe you're mostly correct or on the right track. Like Minko said in a blog post:
So Ivy can be retro compatible with View Engine libraries but can't produce View Engine libraries and library authors might have to think like... Is my library compiled with View Engine? If yes, then it can work on View Engine based or Ivy based apps but depends on Angular version and application engine. Is my library compiled with Ivy? If yes, then it will only work on Ivy based apps. So I agree with @Timebutt, release a new major version ( Note: Ivy is available since Angular 8. On Angular <=8 the default engine is View Engine, on Angular 9+ the default engine is Ivy. View Engine was deprecated on Angular 12 and removed on Angular 13. |
Just to be clear, the @nunoarruda, thanks for all the info. It's very helpful. I'm realizing this PR outputs an Ivy library (because it removes this line and because We need to continue to output a View Engine library if we want our library to be compatible with View Engine Angular projects. We could hypothetically still merge this PR, and reintroduce For the next major release of Unfortunately, I decided to have FullCalendar's packages share the same major version number, to clearly indicate compatibility between FullCalendar plugins. Having an impromptu v6 for In the meantime, |
@Timebutt, I'm trying to get this into the world. In reference to my previous comment, I've decided that, to avoid creating a breaking change in v5, and to avoid creating a new major version that would be out-of-sync with other plugins, we should create a new package called When FullCalendar v6 is released, it will become the standard I've attempted to do this in the ivy branch in this repo. It is building correctly. However, when I try to build this new example project against it, it gives errors like:
For some reason it does not see many of the imports as ESM. Do you happen to know why? |
Hey @arshaw, sorry I didn't mean to come across as impatient when referencing this PR in my comment yesterday ;) I know you are waiting for the next major release to ship the Ivy package format. I have updated my PR yesterday by the way, as there were conflicts and bumped some more dependencies while at it. As far as I know, it should be fully functional but I have not yet tested the changes I did yesterday. I'll get to it later today and report back, or fix the problem should I find one! |
Hey @arshaw, I took a look at the example project you mentioned and figured out the problem. The issue is that you are using version |
@Timebutt I made a PR to your PR :P .... my team and I are just trying this out. It might take some more work but we need this for our Angular 14 project. Timebutt#1 See https://www.npmjs.com/package/@benpsnyder/fullcalendar-angular-ivy |
@benpsnyder what do you mean you need this for your Angular 14 project? I'm currently running |
This PR upgrades this repository to Angular 13. It also migrates everything to
eslint
8.1.0
asTSLint
was dropped from Angular. I verified all commands listed inpackage.json
still work, AFAIK it should be fully functional.Main advantage of this upgrade is that packages can finally be published as
Ivy
packages, removing the need for consuming apps to runngcc
!