fix: bump npm infrastructure dependencies and make sure changing serve.js content is applied#2542
Merged
nedtwigg merged 7 commits intodiffplug:mainfrom Jul 7, 2025
Conversation
this was a hidden update issue: if we change the serve-script content between two releases but the user keeps the package.json the same, the md5 hash used for the node-modules-dir stayed the same, resulting in us not using the latest version of the serve script but keep using the old one. That was especially harmful in the case where we changed the api of the serve-script to accept a uuid to allow multiple instances. The old script just ignored that resulting in a server-process that was never found due to wrong port-file-names written/waited for.
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR ensures the serve.js content is included when hashing node-modules directories and bumps the underlying npm server dependencies.
- Extend MD5 hashing to cover both
package.jsonandserve.jscontents - Update NodeServerLayout and step state to pass through serve script content
- Bump versions of Express and graceful-shutdown packages in all serve scripts
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/src/test/java/com/diffplug/spotless/npm/NpmResourceHelperTest.java | Added unit tests for the new MD5 overload |
| lib/src/test/java/com/diffplug/spotless/npm/NodeServerLayoutTest.java | Added tests to verify the hash changes when serve.js differs |
| lib/src/main/resources/com/diffplug/spotless/npm/tsfmt-serve.js | Fixed typo when sending error messages |
| lib/src/main/resources/com/diffplug/spotless/npm/common-serve.js | Replaced old shutdown manager with http-graceful-shutdown |
| lib/src/main/resources/com/diffplug/spotless/npm/tsfmt-package.json | Bumped server package to v4.0.0 and updated dependencies |
| lib/src/main/resources/com/diffplug/spotless/npm/prettier-package.json | Bumped server package to v4.0.0 and updated dependencies |
| lib/src/main/resources/com/diffplug/spotless/npm/eslint-package.json | Bumped server package to v4.0.0 and updated dependencies |
| lib/src/main/java/com/diffplug/spotless/npm/NpmResourceHelper.java | Extended md5 to accept multiple contents and updated implementation |
| lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java | Pass serve.js content into NodeServerLayout |
| lib/src/main/java/com/diffplug/spotless/npm/NodeServerLayout.java | Revised directory name generation to include serve script hash |
| lib/build.gradle | Added testlib project to testing classpath |
Comments suppressed due to low confidence (5)
lib/src/main/java/com/diffplug/spotless/npm/NpmResourceHelper.java:146
- [nitpick] The variable name
additionalFilecontentStreammixes casing and violates CamelCase conventions. Consider renaming it toadditionalFileContentStreamfor consistency.
Stream<String> additionalFilecontentStream = Stream.concat(
lib/src/main/resources/com/diffplug/spotless/npm/common-serve.js:3
- [nitpick] The imported function is assigned to a generic name. Consider renaming the shutdown handler (e.g., to
shutdownServer) to avoid confusion with system-level shutdown methods.
const gracefulShutdown = require("http-graceful-shutdown");
lib/src/main/resources/com/diffplug/spotless/npm/tsfmt-package.json:12
- Runtime dependencies like
expressandhttp-graceful-shutdownare declared underdevDependencies. If these scripts run in production, consider moving them todependenciesto ensure they're always installed.
"express": "5.1.0",
lib/src/main/resources/com/diffplug/spotless/npm/prettier-package.json:12
- Runtime dependencies like
expressandhttp-graceful-shutdownare declared underdevDependencies. If these scripts run in production, consider moving them todependenciesto ensure they're always installed.
"express": "5.1.0",
lib/src/main/resources/com/diffplug/spotless/npm/eslint-package.json:12
- Runtime dependencies like
expressandhttp-graceful-shutdownare declared underdevDependencies. If these scripts run in production, consider moving them todependenciesto ensure they're always installed.
"express": "5.1.0",
lib/src/main/java/com/diffplug/spotless/npm/NpmResourceHelper.java
Outdated
Show resolved
Hide resolved
lib/src/main/resources/com/diffplug/spotless/npm/common-serve.js
Outdated
Show resolved
Hide resolved
Member
|
Released in |
simschla
added a commit
to simschla/spotless
that referenced
this pull request
Dec 9, 2025
…rs on same node_modules Before this change: if two (or more) node-based servers have been started on the same node_modules dir, the could override each others temporary port files, resulting in some of the servers not correctly launching. With this change: each server process makes sure to write distinct temporary and final files for transporting selected port number to the launching java process. This is a fixup for diffplug#2462. Refs: diffplug#2462, diffplug#2542
simschla
added a commit
to simschla/spotless
that referenced
this pull request
Dec 9, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In #2462 we introduced the support for using the same
node-modules-dir for multiple spotless steps requiring the same dependencies. To achieve this, the serve.js files were updated to accept a UUID when starting a npm-based server so that the java side knows which server-process (which port) it may use.This created a problem we did not foresee: If a user has a running spotless installation before this change, then updates to the version containing this change, it leads to a problematic situation: the existing
node-modules-dir (created by the previous spotless version and thus containing the old serve.js files without the UUID-change) would prevent the new spotless version from using the newserve.jsfiles (containing the UUID-change) because the md5 hash used in thenode-modules-dir's name did only respect the package.json content, not theserve.jscontent.This PR changes this, so that the content of the
package.jsonas well as the content of theserve.jsfiles is respected in order to calculate a md5 hash for naming thenode-modules-dir. So if any of these files change, a newnode-modules-dir is used.This PR also bumps the dependencies used behind the scenes for creating and managing the npm server process.