Skip to content
This repository has been archived by the owner on May 15, 2019. It is now read-only.

Hot reloading does not work for subdirectories in Linux #207

Closed
laurenzlong opened this issue May 10, 2018 · 5 comments · Fixed by #223
Closed

Hot reloading does not work for subdirectories in Linux #207

laurenzlong opened this issue May 10, 2018 · 5 comments · Fixed by #223

Comments

@laurenzlong
Copy link
Contributor

laurenzlong commented May 10, 2018

Environment details

  • OS: Linux
  • Node.js version: 7.11.5
  • npm version: 3.10.10
  • @google-cloud/functions-emulator version: 1.0.0-beta.4

Steps to reproduce

  1. Create a functions folder where index.js is not at the root directory, but in a subdirectory such as 'lib/index.js' (this is a common pattern for users that write functions in TypeScript and then compile them to JS)
  2. Emulate the function
  3. Change the source code in lib/index.js
  4. Observe that the worker is not closed

After taking a look at the code, it seems from this line that fs.watch is used for hotloading. This has a known limitation that recursive: true does not work on Linux (see https://nodejs.org/api/fs.html#fs_caveats). In order to work around it, what do you think about checking package.json for the main field, which indicates the file that contains the functions, and then also watching the directory that file is in? But this would still lead to problems if the main file for functions calls helper functions in other subdirectories, so it doesn't solve the issue completely. Alternatively, the emulator can crawl the entire directory first so it knows all the subdirectories to watch, but that is a lot of work. So perhaps it's worth just printing out a warning messaging when Linux is detected.

@psigen
Copy link

psigen commented May 13, 2018

For what it's worth, fs.watch is somewhat known for this limitation and several third party libraries exist to deal with it.

For example, webpack has a whole high-level API watchpack which wraps a lower-level API chokidar used in several well-known frameworks to perform this exact function in a cross-platform-friendly way. Perhaps it would be useful to integrate with one of them or at least replicate their behavior?

@OrDuan
Copy link

OrDuan commented Jun 19, 2018

I want to add that it's not Typescript specific. Anyone who uses Babel as well won't be able to use the hot reload.
I'm using Babel since I want to use the latest js features on my backend. To verify that the issue is related to the directory path I changed ~/.npm-global/lib/node_modules/firebase-tools/node_modules/@google-cloud/functions-emulator/src/supervisor/worker.js file from this line:

const localdir = getLocaldir(cloudfunction)

To this:

const localdir = getLocaldir(cloudfunction)+'/dist';

And the hot reload indeed works again(no reloading logging, but the function gets updated)

@Harry-Torry
Copy link

I'm developing an app with Angular 6, it's a bit hacky but straight up editing the functions/lib/index.js file works as a workaround to get functions shell to hot reload.

@sdbondi
Copy link
Contributor

sdbondi commented Jul 17, 2018

I added a PR here which uses chokidar and this works when I test hot reloading on one of my projects. Any help with getting this through would be appreciated 😄 I ran the tests locally before making any changes (master) and a lot of them failed/I needed to set something up. I expected circle ci to just run them when I made the PR but that didn't seem to happen. 🤷‍♂️

#223

jmdobry referenced this issue Nov 16, 2018
* Prettier

* Fix(worker): Use chokidar for file watching fixes#207

* fix(worker): Re-added missing debug conditional
@beeman
Copy link

beeman commented Nov 16, 2018

Thanks a lot for landing this ❤️

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

Successfully merging a pull request may close this issue.

7 participants