appsody / stacks

Appsody application stacks. This repo will be archived soon.
https://appsody.dev
Apache License 2.0
89 stars 121 forks source link

nodejs-express stack does not restart upon changes to package.json #583

Open nastacio opened 4 years ago

nastacio commented 4 years ago

Describe the bug Appsody is not restarting the application when package.json is modified.

To Reproduce Steps to reproduce the behavior:

  1. Create a new application with appsody init nodejs-express
  2. Start the application with appsody run
  3. Make an update to package.json in the root folder of the application directory

Expected behavior Appsody should restart the application

Actual behaviour Nothing happens, the application is not started

Environment Details (please complete the following information):

If applicable please specify:

Screenshots N/A

Additional context I can see the following environment variables in the container and package.json is not in APPSODY_WATCH_REGEX

# set
APPSODY_DEBUG='npm run debug'
APPSODY_DEBUG_KILL='true'
APPSODY_DEBUG_ON_CHANGE='npm run debug'
APPSODY_DEPS='/project/user-app/node_modules'
APPSODY_MOUNTS='/:/project/user-app'
APPSODY_PREP='npm install --prefix user-app && npm audit fix --prefix user-app'
APPSODY_PROJECT_DIR='/project'
APPSODY_RUN='npm start'
APPSODY_RUN_KILL='true'
APPSODY_RUN_ON_CHANGE='npm start'
APPSODY_TEST='npm test && npm test --prefix user-app'
APPSODY_TEST_KILL='false'
APPSODY_TEST_ON_CHANGE=''
APPSODY_WATCH_DIR='/project/user-app'
APPSODY_WATCH_IGNORE_DIR='/project/user-app/node_modules'
APPSODY_WATCH_REGEX='^.*.js$'
sam-github commented 4 years ago

I've noticed this, too, though with respect to the template files. Also, if typescript is used (*.ts) I don't think it will be restarted.

In your case, what changes are you making? What do you think will happen when the project is restarted?

Note the restart command is APPSODY_RUN_ON_CHANGE='npm start', but the start command that is run is the one from inside the appsody project's package.json. I can't think of anything in the user-project's package.json that would change how npm start works, so I'm not sure why adding it to the monitored files would be helpful.

nastacio commented 4 years ago

In your case, what changes are you making?

Modify the contents of "devDependencies". If you modified the .js files first to add the require statements, the application is restarted without the dependencies and fails, then when you add the dependencies to the package.json file, nothing happens.

Obviously, doing the file modifications in the reverse order masks the problem.

sam-github commented 4 years ago

I can't speak for the appsody designers, but I don't think there is a good way to fix this ATM.

Adding package.json to the file watch would "restart" the app, but restarting is just npm start, and what you need is npm install; npm start after the package.json dependency specs change. That could be the restart command... but it would be much slower than just npm start to call npm install; npm start every time when only a .js file changed.

nastacio commented 4 years ago

There is no technical constraint on using something other than single npm sub-commands.

It should be possible to create a script that makes more fine-grained comparisons and decisions.

sam-github commented 4 years ago

It should be possible to create a script that makes more fine-grained comparisons and decisions.

I'm not sure that it is possible. Creating a script that did npm install only when the package.json file changed would require some way to know what files had changed.

@Kamran64 Is the appsody file watcher capable of making available to the APPSODY_RUN_ON_CHANGE command the list of changed files?

Kamran64 commented 4 years ago

@sam-github @nastacio - We don't currently have this capability today. I agree, the ON_CHANGE variables being customised to run different commands depending on the file changed would be useful though.

Thinking about this issue, how much extra time would running npm install followed by npm start add on to the current recompile time if just a js file was changed compared to a package.json? I assume it would only take much longer if a dependency was added to the package file so the extra time might be worth it to avoid problems like this cropping up. What do you think @sam-github?

nastacio commented 4 years ago

@Kamran64 the capability is ON_CHANGE being able to call anything that may be required. Always calling npm install would not be a good approach, it would have to be conditionally called if something like a checksum on package.json differed from the checksum of the package.json file extracted on the last time npm install was invoked.

sam-github commented 4 years ago

So, this isn't currently an appsody feature.

@nastacio the stacks are customizable, perhaps you can take a shot at this if its high-value for you?

nastacio commented 4 years ago

@sam-github would you be ok if I submitted a PR along the lines of my previous comment: new script that records some sort of signature for package.json upon calling npm install and then only calls it again if the signature changes?

I am not certain I will find the time to submit it (not really working with the stack full time (it was just notes of things I found while writing a tutorial) , but wanted to make sure it is inline with what the stack authors would do if needed.

sam-github commented 4 years ago

I don't consider this a deficiency specific to the nodejs stacks. While I'm most familiar with them, it must surely be a general problem that certain types of setup are time consuming enough they are not done on every stack restart.

For that reason, I'm not convinced that patching it into the nodejs stacks is a good idea without it being a feature generally supported by appsody.

While I've been tapped to maintain the nodejs stacks, I'm not an appsody team member, I won't speak to their architectural choices. This is a question for @appsody.