Snowpack: Add back support to reinstall linked dependencies on change

Created on 22 Dec 2020  路  4Comments  路  Source: snowpackjs/snowpack

Original Discussion: https://github.com/snowpackjs/snowpack/issues/429
/cc @joaotavora

Regression:

commit 610fb9d28452bf24758387a00fe81a4ed103e503
diff --git a/packages/snowpack/src/commands/dev.ts b/packages/snowpack/src/commands/dev.ts
--- a/packages/snowpack/src/commands/dev.ts
+++ b/packages/snowpack/src/commands/dev.ts
@@ -954,3 +880,3 @@
   function onDepWatchEvent() {
-    reinstallDependencies().then(() => hmrEngine.broadcastMessage({type: 'reload'}));
+    hmrEngine.broadcastMessage({type: 'reload'});
   }
bug v3

Most helpful comment

After much rummaging around in the code, I've settled for exporting and re-using an function in sources/local instead of bringing back the defunct reinstallDependencies(). I thus touch very few lines, which is always a good idea in my book. Maybe/probably this misses some important detail but it works. Getting late now, going to bed.

All 4 comments

I'm working on a fix right now :-) Having fun, but I won't be suprised if you beat me to it, since they'll be my first ever lines of OSS javascript/typescript.

Nice! Let me know how I can help. That commit should include the old implementation of reinstallDependencies(), which may need to change a bit since it's so old now, but should have enough to point you in the right direction. Appreciate the help!

I'm making progress, indeed. I'll let you know if I am blocked. First I'll try to get back that function you mention and get this working the way it presumably used to.

After that is done, I was thinking of being a little more ambitious and installing only the dependency that we know has changed, since I think reinstallDependencies installs _all_ of them. In theory it should be possible to know which one we need to update by correlating the argument to onDepWatchEvent which is the file/directory that changed with some other global structure.

After much rummaging around in the code, I've settled for exporting and re-using an function in sources/local instead of bringing back the defunct reinstallDependencies(). I thus touch very few lines, which is always a good idea in my book. Maybe/probably this misses some important detail but it works. Getting late now, going to bed.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

wenyanqi picture wenyanqi  路  5Comments

backspaces picture backspaces  路  5Comments

FredKSchott picture FredKSchott  路  6Comments

gr2m picture gr2m  路  6Comments

FredKSchott picture FredKSchott  路  6Comments