angular-skyhook
angular-skyhook copied to clipboard
ivy runtime error
Skyhook works perfect with the actual default angular compiler, but when you change the compiler to ivy, you get this runtime error.
It seems to be a problem with the directives and ivy.
https://github.com/angular/angular/issues/25813
Ivy is meant to be backwards compatible, although I am sure there must be caveats, as nothing in JavaScript is actually that easy. In any case, I'm going to assume Ivy-related bugs are on Ivy's side until convinced otherwise, and do close to zero investigation. You can leave it open in case anyone else comes across it.
the injection context is a breaking change. however if someone wants to use your existing build they run the basic postinstall script described here on a project that depends on angular-skyhook, it should work: https://next.angular.io/guide/migration-ngcc
"postinstall": "ngcc --properties es2015 browser module main --first-only --create-ivy-entry-points"
this should do all the conversion for anyone using the library.
That said, when you do that the other postinstall script they mention, the for use with nguniversal,
"postinstall": "ngcc --properties main --create-ivy-entry-points"
fails if using @angular-skyhook/dnd-multi-backend due to https://github.com/angular/angular/issues/33697
This and a number of other number of breaking changes ng update
will fix for you and leave the library in a state that is still backwards compatible.
This is great, thank you.
Once you pull in the dnd-multi-backend v5 update the ngcc error should go away because the rollups are better formatted. That doesn't make it natively ivy compatible, but it will make it useable. When is that branch going to be ready? Could you make a beta release off that branch for testing purposes? It would be very helpful.
1.4.0-rc.0
#507
I found the problem with ivy. In Ivy you have to inherit from another directive. So your abstract class DndDirective doesn't work with ivy, if you adapt it to a real Directive it should be ivy compatible, see here:
https://next.angular.io/guide/ivy-compatibility-examples#undecorated-classes
In my fork it is fixed: https://github.com/Corfus/-angular-skyhook-ivy/tree/cdb/ivy-compatibility-fix
Is that all that needed to be done?
In any case, can you make a PR? A fork is cool, but a PR actually helps land the fix.
Yes with this adaptions it works without distinctions with ivy, too.
I made the PR. https://github.com/cormacrelf/angular-skyhook/pull/532
@cormacrelf when do you think it will be merged?
I think I’ll wait about 2 years, during which time I will get to some of the little things I’ve always wanted to do, like cultivate a tasteful indoor-plant aesthetic, whittle a log into the shape of a realistic Christmas ham, and train as a spider milker at the local zoo.
I think I’ll wait about 2 years, during which time I will get to some of the little things I’ve always wanted to do, like cultivate a tasteful indoor-plant aesthetic, whittle a log into the shape of a realistic Christmas ham, and train as a spider milker at the local zoo.
A somewhat exaggerated answer to a question without bad intention.
Good luck and follow your dreams.
I had hoped that merging the PR immediately would let me joke about it, but apparently no. Back to being a maintenance robot it is.
Did I miss the release?
Yay angular 9 is out! finally! I'm sticking with ViewEngine for a few months before switching to Ivy, but following this thread to see how much spider milk @cormacrelf has made. I'd like to see a picture of the ham log, too.
Any update to this? ngcc
advice from @aaronmanela did not help to me and I found it impossible to use the skyhook with Angular 9 (with IVY enabled) so far 😞
Did you try using 1.4.0-rc0 branch @palpatine1991 ?
Also the ngcc postinstall fixes will no longer be recommended in Angular 9.1 as they have made it better.
Yes, I am using skyhook 1.4.0-rc0 and I have tried it even with Angular 9.1.0. Still have the same error
@cormacrelf do you plan to update examples to Angular 9.* with Ivy enabled? The "@angular-skyhook/core": "^1.4.0-rc.0"
does not work with Ivy enabled for me.
@cormacrelf I have made a PR which fixes the library for IVY. Could you please review it, happy to take any feedback on it, you will see it is very basic fix to make the ngcc compiler happy. https://github.com/cormacrelf/angular-skyhook/pull/571
@acb122 is the core of your fix adding the missing exports, upgrading dependencies or disabling the IVY?
I mean if IVY disabling is not a core of the fix, I would consider to keep it on because of future maintainability
@palpatine1991 the core fix is the missing exports.
IVY disabled is the current recommended setting in angular libraries in angular v10 that will change.
I have removed my upgrading angular change as that broke tests.
Hey @cormacrelf, first of all, thanks for maintaining this library! I was wondering when will you be able to merge PR #571 made by @acb122 ? We're having problems with Angular 9 and this library!
Thanks again!
Is there any update on this? Have anyone published #571 to new NPM package
@palpatine1991 @acb122 @aaronmanela @MarioC3 @a7md0 I have published the fixed version to npm, everything is fine with angular ivy. @ng-dnd/core @ng-dnd/multi-backend @ng-dnd/sortable