noppa / ng-hot-reload

Hot reloading for AngularJS apps.
MIT License
43 stars 8 forks source link

Unable to get hot reloading #24

Closed gincher closed 4 years ago

gincher commented 4 years ago

Hi. Every time I make any change, in the template, in the component code or in the controller, it reloads the page. There are no errors or messages in the console - it feels just like it was before adding the loader. What am I doing wrong?

I've added the loader to the loaders array of all js files, in the index of 1 (0 is cache loader). I added 'webpack-dev-server/client?http://localhost:8181', 'webpack/hot/only-dev-server', to the entry array (8181 is the port i'm using in the devServer).

My components and directives are initiated by exporting a function, and in a main file, the function is triggered, for example:

import SomeComponentTemplate from './SomeComponentTemplate.html';

const SomeComponent = {
    template: SomeComponentTemplate,
    controller: 'SomeComponentController',
};

export SomeComponentLoader = (module) => module.component('someComponent', SomeComponent);

(This is a hack for letting 'angularjs-annotate' babel plugin auto annotate the code, it looks for .component().

I'm using react hot module reloading (the root is angular, but some components are react) and it works perfectly, so it's not something with webpack dev server.

Thanks!

noppa commented 4 years ago

What does the main file where you register these files look like?

Unlike (AFAIK) React hot loader, this library doesn't do any static analysis to dectect and modify components. Instead, it modifies angular so that calls to angular.module('yourModuleName').component(..) will be directed to the hot reloading mechanism of this library instead of angular's normal component call. For that all to work, there needs to be angular.module('yourModuleName') call somewhere for this library to do its thing.

You'd get the best results if each of the component files would register the component themselves, like

import SomeComponentTemplate from './SomeComponentTemplate.html';
const SomeComponent = {
    template: SomeComponentTemplate,
    controller: 'SomeComponentController',
};
angular.module('yourModuleName').component('someComponent', SomeComponent)

The problem here is that when that component file of yours is updated, it doesn't actually register any components itself, so ng-hot-reload will just assume that it's not a component file and won't capture the update.
However, that might not be an absolute showstopper, because Webpack then propagates the update up the import dependency chain to the file where you import these component files and (I assume) call SomeComponentLoader(angular.module('yourModuleName')) or just SomeComponentLoader(yourModuleInstance).

If it's the latter, where yourModuleInstance is a static reference to a variable that holds your module, then that won't really work at all here because yourModuleInstance is still the same module instance that it was before, not a special module decorated by this library.

I suggest you do something like

// module.js
const myModuleName = 'myModule'
angular.module(myModuleName, [dependencies])

// Export the name of the module, not the module instance
export default myModuleName

// main.js
import moduleName from './module.js'
import SomeComponentLoader from './someComponent.js'

const myModule = angular.module(moduleName)

SomeComponentLoader(myModule)

This still has the problem that now all the components that are registered in main file will be updated, not just the one that was actually changed, but at least myModule should be decorated by this library so that might work.

Also not that you can't require angular in the main file but will have to rely on the global angular reference, or this library won't be able to override the reference. If you absolutely can't do that and want to import angular in the file, you might want to ditch the webpack loader and use babel-plugin-ng-hot-reload which AFAIK does handle angular imports. It's not my project but seems to be built on top of the core package of this library.

gincher commented 4 years ago

Hi, thanks for the reply. I did some digging just to try something out, I know that it will not refresh the components, (I'll need to re-run the SomeComponentLoader function if I understood correctly)

const loader = require('ng-hot-reload-core');
const hotReloadingAngular: typeof angular = loader.decorateAngular({
    angular,
    forceRefresh: true,
    preserveState: true,
});

Some of the components load correctly, but in others, this is an empty object even thought I declared binding in the component and I'm sure that a value was passed. When changing hotReloadingAngular with angular, everything works.

noppa commented 4 years ago

I thought about this a bit today and I'm thinking that your setup with the custom loader functions is probably not common enough that it ought to be added as an option to ng-hot-reload-loader.

Instead, I think you should implement custom webpack loader for your project, using ng-hot-reload-core. This way you can make it work well with your specific project setup.

I made a quick example project here: noppa/ng-hot-reload-custom-loader-example. The loader implementation can be found in hot-loader.js.

The idea there is to look for exported functions that take argument named "module" and register components or controllers with it and then ensure that the function is called with the mocked hot-reloading version of angular.

It's super hacky and brittle, but hey seems to work pretty well for development purposes so who cares.

gincher commented 4 years ago

I can't even begin to explain how much I appreciate you spending the time to help me. This loader does work and it's amazing to see the hot reload, thank you very much.

But I do still have a problem with the controllers having this empty. If I do console.log({...this}); setTimeout(() => console.log({...this}), 1000);, the first log will show an empty object, while the second one will have all the bindings (props). The problem is that in many very old controllers we are doing this:

function UserCtrl($scope) {
    const ctrl = this;
    $scope.data = {
        userId: ctrl.user.id
    };

    // ...
}

It's like doing const c = new UserCtrl(); Object.entries(bindings).forEach(([key, value]) => c[key] = value); instead of UserCtrl.call({...bindings});.

I understand that this is not the right way to do it, but so does using angularjs 😄- it's legacy code that will take too much time to change.

Again, thank you very much!

noppa commented 4 years ago

No problem, I'm glad it's at least getting close to working for your setup and helping your develpment experience.

Pre-assigning bindings to the controller before constructor is called was actually deprecated in AngularJS 1.5 and removed in 1.7. In later versions of AngularJS you'd need to put that code inside $onInit.

What version of AngularJS are you using? I can take a look if we could add support for it without too much work. Although you should probably also consider migrating to a newer version of AngularJS and the use of $onInit, so this

function UserCtrl($scope) {
    const ctrl = this;
    $scope.data = {
        userId: ctrl.user.id
    };
}

would just become

function UserCtrl($scope) {
    this.$onInit = () => {
      const ctrl = this;
      $scope.data = {
          userId: ctrl.user.id
      };
    };
}

I think it would be pretty easy to create a jscodeshift codemod for that transform so you could automate most of the upgrade work.

gincher commented 4 years ago

I'm using angular 1.5, but many components were built before the onInit hook was added to angular.

The problem that I have with moving the data object to $onInit is that the $onChanges hook is triggered before, many components need to modify or access the data object. Also, some components implement their own init hook (just calling the init function at the end of the function component), which might be triggered before the real $onInit hook will be triggered.

noppa commented 4 years ago

I think I found a solution for the issue and published a new prerelease version with the fix.
Try installing ng-hot-reload-core@3.2.0-alpha.0 to use with your loader and see if that works better.

gincher commented 4 years ago

It works! Thank you very much!