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

Restart the app if environment variables change #389

Merged
merged 1 commit into from
Sep 5, 2018

Conversation

quis
Copy link
Member

@quis quis commented May 3, 2017

Could be confusing why your changes aren’t having any effect.

Someone should probably test this before merging it 😬

cc @edwardhorsford

@gemmaleigh
Copy link
Contributor

I tested this locally, when I make changes to the .env file and save them, nodemon doesn't restart the app.

@quis
Copy link
Member Author

quis commented May 8, 2017

Thanks for the testing.

I’ve changed it so that it actually works in b7bf675:

Doing this using the existing file-extension-based config doesn’t work because nodemon doesn’t undestand that .env is a file with no name, and an extension of env. So I’ve removed the file-extension-based stuff entirely and rewritten it to use globbing.

Tested locally that:

  • changing server.js or .env causes the app to restart
  • changing a file inside node_modules doesn’t cause the app to restart

@NickColley
Copy link
Contributor

I have pulled Chris' change locally and confirmed that changing a file ending with .js, .json and .env triggers nodemon to restart the application.

@NickColley
Copy link
Contributor

I think all we need is an addition to the CHANGELOG under 'features'

@quis
Copy link
Member Author

quis commented Aug 9, 2018

@NickColley done and squashed into aaf531c

Copy link
Contributor

@NickColley NickColley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweet thanks Chris

@joelanman
Copy link
Contributor

Quick thought, does this affect very first start up after install? The kit creates empty .env at that point, wonder if that triggers restart and if that's a problem

@NickColley
Copy link
Contributor

NickColley commented Aug 10, 2018

@joelanman no problems from my testing.

  1. delete .env
  2. run npm start
  3. edit .env

only restarts once when the edit is made.

Copy link
Contributor

@joelanman joelanman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks! Do you mind rebasing, as there's now a conflict with CHANGELOG

Could be confusing why your changes aren’t having any effect.

Doing this using the existing file-extension-based config doesn’t work
because nodemon doesn’t undestand that `.env` is a file with no name,
and an extension of `env`. So I’ve removed the file-extension-based
stuff entirely and rewritten it to use globbing.

Tested locally that:
- changing `server.js` or `.env` causes the app to restart
- changing a file inside `node_modules` _doesn’t_ cause the app to
  restart
@36degrees
Copy link
Contributor

Rebased against master.

@NickColley
Copy link
Contributor

👍 to merge

@36degrees 36degrees merged commit 17e313a into alphagov:master Sep 5, 2018
@36degrees
Copy link
Contributor

Thanks, @quis!

@kr8n3r kr8n3r mentioned this pull request Sep 10, 2018
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

Successfully merging this pull request may close these issues.

5 participants