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 dev server on package.json changes #5412

Merged
merged 4 commits into from
Nov 16, 2022
Merged

Conversation

matthewp
Copy link
Contributor

Changes

Testing

  • Unit test added which confirms the dev server is restarted when the package.json changes.

Docs

N/A, bug fix

@changeset-bot
Copy link

changeset-bot bot commented Nov 15, 2022

🦋 Changeset detected

Latest commit: 4ceb81a

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added pkg: astro Related to the core `astro` package (scope) pkg: solid Related to Solid (scope) pkg: integration Related to any renderer integration (scope) labels Nov 15, 2022
Copy link
Member

@natemoo-re natemoo-re left a comment

Choose a reason for hiding this comment

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

LGTM! One comment about being a bit more defensive.

Comment on lines +30 to +32
if(cwd) {
watchFiles.push(fileURLToPath(new URL('./package.json', pathToFileURL(cwd))));
}
Copy link
Member

Choose a reason for hiding this comment

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

This seems like an odd edge case, but should we check if package.json exists first?

Since Deno is landing npm: support, I think we're not far off from an Astro project that doesn't have a package.json file being a feasible thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't need to check for that. This is used that when a file does change and it matches one of the files in watchFiles, it will trigger a restart. If the file doesn't exist then it will never match. It doesn't cause some other process to try and read the (potentially not existing) files.

Here's the only place this information is used:

shouldRestart = settings.watchFiles.some(
(path) => vite.normalizePath(path) === vite.normalizePath(changedFile)
);

@matthewp matthewp merged commit a278c7a into main Nov 16, 2022
@matthewp matthewp deleted the reload-on-package-changes branch November 16, 2022 13:34
@astrobot-houston astrobot-houston mentioned this pull request Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope) pkg: integration Related to any renderer integration (scope) pkg: solid Related to Solid (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HMR: Installing sass when in error state does not reload page
2 participants