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

[feature] Add support for hot reloading #14

Open
c12i opened this issue Nov 18, 2024 · 8 comments
Open

[feature] Add support for hot reloading #14

c12i opened this issue Nov 18, 2024 · 8 comments

Comments

@c12i
Copy link
Contributor

c12i commented Nov 18, 2024

This is a follow up from the original spike holochain/scaffolding#394

Here are some findings I made that could make this achievable:

  • Configuration: hot reloading could be configured with a flag and an option in the hc-spin cli: --hot-reload would enable hot reload and --watch-dir would instruct the file watcher which directory/ glob to watch for changes for example the ./dnas dir.
  • Integrating a file watcher. In node.js, chokidar looks like a very good option and it provides cross-platform support.
  • Putting it all together:
  if (cli.opts().hotReload) {
    const watcher = chokidar.watch(cli.opts().watchDir, { persistent: true });
    watcher.on('change', async (file) => {
      console.log(`${file} has changed`);
      childProcess.execSync('cargo build --release --target wasm32-unknown-unknown');
      childProcess.execSync('hc app pack workdir --recursive');
      // more steps
    });
  }

The code snippet above shows how we can initialize a file watcher and listen for change events where the hot reloading logic would live. From my understanding, this would be the correct sequence of events, feel free to correct me if I miss anything:

  • Rebuild wasm
  • Pack app
  • Save newly built happ or webhapp
  • Via admin ws: disable previously running app
  • Via admin ws install new app

I have only been able to test out the the first two steps, I ran into some issues calling the rustUtils function to save the happ or webhapp, I believe you have better understanding on how we would go about with the next steps or if there's anything I missed there that should probably be done too.

Some caveats with the proposed approach though, there is still no way to ensure that saved data is persisted between hot reloads, I think this can be a problem we tackle later one though once we get hot reloading working. Additionally, we cannot guarantee that the dna hash will remain the same across hot reloads, I am not sure this would be a problem while developing though, but this can be worked around by specifically setting the --watch-dir as the path to the coordinator zome source code.

Let me know what you think.

@matthme
Copy link
Collaborator

matthme commented Nov 18, 2024

You'd also need logic to reconnect to the app websocket in the frontend. It will use an authentication token that was bound to an app that does not exist anymore so the UI should start failing I think. I don't know how to solve that part without adding some special logic to the js-client which I think seems problematic.

@matthme
Copy link
Collaborator

matthme commented Nov 18, 2024

Not sure though...maybe a page reload is enough and the js-client would read the new parameters from the window object.

@matthme
Copy link
Collaborator

matthme commented Nov 18, 2024

But they would need to be updated there somehow. This logic of writing connection parameters to the window object is currently part of the preload script I think.

@c12i
Copy link
Contributor Author

c12i commented Nov 19, 2024

You'd also need logic to reconnect to the app websocket in the frontend. It will use an authentication token that was bound to an app that does not exist anymore so the UI should start failing I think. I don't know how to solve that part without adding some special logic to the js-client which I think seems problematic.

Hmmm, I see, the running app would need the authentication token updated and the window reloaded to reflect this change. Triggering a page reload programatically should be achievable.

I think what we can try figure out is whether it's possible to programatically update the preloadScript that sets this token

@matthme
Copy link
Collaborator

matthme commented Nov 19, 2024

I think the preload script is only run once when the browser window is opened...so we would probably need to find another way to solve this. Maybe by adding an event listener to the preload script and then emit an event to the window or something like that.

@matthme
Copy link
Collaborator

matthme commented Nov 19, 2024

Well, since the data is going to be whiped anyway, maybe the way to go would just be to restart the browser windows?

@matthme
Copy link
Collaborator

matthme commented Nov 19, 2024

That's how I know it from electron frameworks as well if you change the nodejs backend code.

@c12i
Copy link
Contributor Author

c12i commented Nov 19, 2024

Well, since the data is going to be whiped anyway, maybe the way to go would just be to restart the browser windows?

Ahh yes this would be better, would restarting close then re-open the currently open windows though?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants