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

❓[SUPPORT]: Angular 11 already supports hmr, is ngxs considering following up? #1711

Open
Y2zz opened this issue Dec 17, 2020 · 24 comments
Open
Labels
discussion An issue that discusses design decisions. good first issue Good for newcomers investigate Requires some investigation not an issue An issue logged that doesn't require fixing (maybe just a question)

Comments

@Y2zz
Copy link

Y2zz commented Dec 17, 2020

https://blog.angular.io/version-11-of-angular-now-available-74721b7952f7

@Y2zz Y2zz added discussion An issue that discusses design decisions. good first issue Good for newcomers investigate Requires some investigation not an issue An issue logged that doesn't require fixing (maybe just a question) labels Dec 17, 2020
@mb590721
Copy link

mb590721 commented Jan 5, 2021

We are very interested in this feature too, HMR is a key to faster development on huge web application.
Any idea if this is planned ?

@markwhitfeld
Copy link
Member

I have asked @splincode to look into it.
We will have some feedback soon about re-introducing this plugin.

@splincode
Copy link
Member

@Y2zz works for me, if something doesn't work for you, please provide an example to reproduce

@krtek4
Copy link

krtek4 commented Feb 15, 2021

Hello,

I had a HMR setup that worked well with Angular 10 using the ngxs/hmr-plugin. After migrating to Angular 11, without changing anything, I had the following error : [HMR] Update failed: Error: Injector has already been destroyed.

After removing the code related to the HMR plugin, the error was gone, but my states weren't saved through reload.

What are the steps to make everything work as before ?

@markwhitfeld
Copy link
Member

@krtek4 Is it possible to provide a small reproduction of your issue?
@splincode could you have a look into this?

@krtek4
Copy link

krtek4 commented Feb 15, 2021

@markwhitfeld not sure it is an issue per se, probably more of a "how to do things now that Angular has HMR"

Not on my work computer right now and stackblitz doesn't seem to properly support HMR, will try to get back to you in the next few days with a repo.

@krtek4
Copy link

krtek4 commented Feb 18, 2021

Hello @splincode , @markwhitfeld

I created a repository with various branches : https://github.com/krtek4/hmr-tests

  • mastger : HMR correctly reload the page, but the state are reset to their default
  • hmr-plugin-10 : the "working version" with Angular 10 where the state remains between HMR refreshes
  • hmr-plugin-11 : HMR plugin is activated with Angular 11 this time, the page goes blank upon refresh and there's an error in the console
  • hmr-plugin-11-bis : HMR stuff in main.ts are removed, HMR correctly reload the page but the state is reset

The steps to reproduce are in the README.md file each time, and in the pull requests. You can also see that the changes between the branches are minimal each time and just related to HMR and the plugin.

Happy to help further if I can.

@krtek4
Copy link

krtek4 commented Mar 15, 2021

Hey @splincode , @markwhitfeld,

Any news on this ? I can probably work on this if you don't have time, just that I don't want to waste time if you already have something planned.

Just tell me how I can help :)

@splincode
Copy link
Member

@krtek4 have not watched yet, you can study this moment yourself

@Y2zz
Copy link
Author

Y2zz commented Mar 15, 2021

I think this may be different from what we understand HMR

@krtek4
Copy link

krtek4 commented Mar 15, 2021

@Y2zz what do you mean ?

HMR itself works fine in Angular 11, with or without NGXS.

The thing that changed is that in Angular 10 and below, using the HMR plugin, data in the stores were restored upon reload, now the plugin does not work, for logical reasons.

What is your issue ?

@splincode
Copy link
Member

@krtek4
Copy link

krtek4 commented May 3, 2021

It's funny to have a comment on this today as I spent the morning trying to fix the issue for me :)

I have a "kinda working setup" currently:

So in a way, it works just fine.

The only issue I have to solve now is Object instances. The storage plugin serialise data as JSON, and thus when they are deserialied, the aren't objects anymore, just data.

Is there any interest from the community to "rewrite" the HMR plugin to manage this issue ? Or write another Storage plugin to keep the object instances ? what do you think is the best option ?

@splincode
Copy link
Member

@krtek4 persist object into storage not good idea, because HMR should be work in only runtime

@krtek4
Copy link

krtek4 commented May 3, 2021

@splincode fair enough, but it is the recommended workaround on the plugin page ;)

Also, I set up in such a way that it only loads when dev mode is active.

I would say the better approach would be to rewrite the HMR plugin to keep the lifecycle part alongside the persistence of the state but letting Angular manage the HRM part. What do you say ?

@splincode
Copy link
Member

In the first versions our hmr plugin, we did so (keep the lifecycle part alongside the persistence of the state), but then we realized that this was an overhead

@krtek4
Copy link

krtek4 commented May 3, 2021

So what would be your proposed approach ?

@splincode
Copy link
Member

splincode commented May 3, 2021

we are waiting for the Angular team to fix the hmr (Ivy)

@krtek4
Copy link

krtek4 commented May 3, 2021

It works outside of JIT, slowly, but it works.

I think the discussion around how should ngxs will manage now that Angular 11 has HRM in the core can already take place as I don't see how any bugfix on their side will change something.

What am I missing ?

On the other hand, I am not the one that opened this issue, and I can implement a solution on my own, just wanted for it to benefit everyone. So if you say you prefer to wait, it works for me too :)

@markwhitfeld
Copy link
Member

@splincode what would we still need in order in terms of angular support to re-introduce this plugin?
Is the Angular 11 HMR support sufficient for us to add this back?

@krtek4
Copy link

krtek4 commented May 3, 2021

FYI, I was able to add some kind of hackish solution to restore the state upon HMR reload base on this old blitz : https://stackblitz.com/edit/angular-hmr-wth-ngxs

  • I'm adding a onDestroy handler on the NgModuleRef that saves the current StateStream on the window object
  • The AppModule then check if a StateStream is present and restore the state from it

Couldn't use the blitz code directly as the module reference is already destroy in Angular 11.

I can share the code if someone is interested.

@splincode
Copy link
Member

need investigate

@oleksandr-codefresh
Copy link
Contributor

@markwhitfeld @splincode any updates about the case?
angular 12/13 still supports hmr

@ylemoigne
Copy link

Still issue with Angular 15 and ngxs hmr plugin ; also the doc say it will be deprecated in angular 10 (https://www.ngxs.io
/plugins/hmr), but it's still alive.

When reload happen :

[HMR] Update failed: Error: NG0205: Injector has already been destroyed.
    at R3Injector.assertNotDestroyed (http://localhost:4200/vendor.js:634550:13)
    at R3Injector.get (http://localhost:4200/vendor.js:634468:10)
    at get applicationRef [as applicationRef] (http://localhost:4200/node_modules_ngxs_hmr-plugin_fesm2015_ngxs-hmr-plugin_js.js:161:35)
    at HmrManager.cloneHostsBeforeDestroy (http://localhost:4200/node_modules_ngxs_hmr-plugin_fesm2015_ngxs-hmr-plugin_js.js:196:27)
    at HmrManager.createNewModule (http://localhost:4200/node_modules_ngxs_hmr-plugin_fesm2015_ngxs-hmr-plugin_js.js:187:33)
    at http://localhost:4200/node_modules_ngxs_hmr-plugin_fesm2015_ngxs-hmr-plugin_js.js:263:17
    at Object.dispose (http://localhost:4200/runtime.js:1104:36)
    at http://localhost:4200/runtime.js:625:41
    at Array.forEach (<anonymous>)
    at internalApply (http://localhost:4200/runtime.js:624:21)
module.exports @ log.js:26
(anonymous) @ dev-server.js:59
invoke @ zone.js:372
run @ zone.js:134
(anonymous) @ zone.js:1275
invokeTask @ zone.js:406
runTask @ zone.js:178
drainMicroTaskQueue @ zone.js:585
Promise.then (async)
nativeScheduleMicroTask @ zone.js:561
scheduleMicroTask @ zone.js:572
scheduleTask @ zone.js:396
scheduleTask @ zone.js:221
scheduleMicroTask @ zone.js:241
scheduleResolveOrReject @ zone.js:1265
resolvePromise @ zone.js:1202
(anonymous) @ zone.js:1118
(anonymous) @ zone.js:1134
self.webpackHotUpdateng_gmp @ jsonp chunk loading:88
(anonymous) @ runtime.adcc7e6bc97e9a60.hot-update.js:2
zone.js:182 Uncaught Error: NG0205: Injector has already been destroyed.
    at R3Injector.assertNotDestroyed (core.mjs:8076:19)
    at R3Injector.get (core.mjs:7983:14)
    at StateFactory.invokeActions (ngxs-store.js:1509:59)
    at MergeMapSubscriber.project (ngxs-store.js:1448:25)
    at MergeMapSubscriber._tryNext (mergeMap.js:44:27)
    at MergeMapSubscriber._next (mergeMap.js:34:18)
    at MergeMapSubscriber.next (Subscriber.js:49:18)
    at FilterSubscriber._next (filter.js:33:30)
    at FilterSubscriber.next (Subscriber.js:49:18)
    at InternalActions.next (Subject.js:39:25)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion An issue that discusses design decisions. good first issue Good for newcomers investigate Requires some investigation not an issue An issue logged that doesn't require fixing (maybe just a question)
Projects
None yet
Development

No branches or pull requests

7 participants