Do not run npm install if generated package json content has not changed #6385
Conversation
Stores hash of the generated package json file in the main package json (inside node_modules, outside target folder) and updates it every time when the generated content is changed. TaskUpdatePackages modified status is changed only if the hash value is changed. It allows do not run npm install even if the generated package.json file is regenerated. Fixes #6151
project-bot
bot
added this to External Reviews
in OLD Vaadin Flow ongoing work (Vaadin 10+)
| digest.digest(content.getBytes(StandardCharsets.UTF_8))); | ||
| } catch (NoSuchAlgorithmException e) { | ||
| // Unrecoverable runime exception, it may not happen | ||
| throw new RuntimeException( |
There was a problem hiding this comment.
|
SonarQube analysis reported 1 issue
Watch the comments in this conversation to review them. |
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1.
Reviewable status: 4 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @denis-anisimov)
flow-server/src/main/java/com/vaadin/flow/server/frontend/NodeUpdater.java, line 262 at r1 (raw file):
} String writeAppPackageFile(JsonObject packageJson) throws IOException {
While this is an API change, I don't think NodeUpdater is in the public API in any meaningful way. What I do wonder is:
should we also add the return value to writeMainPackageFile, which is a direct mirror to writeAppPackageFile - it would keep the methods consistent with each other.
flow-server/src/test/java/com/vaadin/flow/server/frontend/NodeUpdatePackagesTest.java, line 265 at r1 (raw file):
packageUpdater.execute(); System.out.println("xx " + getPackageJson(appPackageJson));
Print should be removed
flow-server/src/test/java/com/vaadin/flow/server/frontend/NodeUpdatePackagesTest.java, line 287 at r1 (raw file):
packageUpdater.execute(); System.out.println("xx " + getPackageJson(appPackageJson));
Print should be removed
There was a problem hiding this comment.
Dismissed @vaadin-bot from a discussion.
Reviewable status: 3 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @ujoni)
flow-server/src/main/java/com/vaadin/flow/server/frontend/NodeUpdater.java, line 262 at r1 (raw file):
API change: no since the method is package local.
should we also add the return value to writeMainPackageFile
Done.
flow-server/src/test/java/com/vaadin/flow/server/frontend/NodeUpdatePackagesTest.java, line 265 at r1 (raw file):
Previously, ujoni (Joni) wrote…
Print should be removed
Done.
flow-server/src/test/java/com/vaadin/flow/server/frontend/NodeUpdatePackagesTest.java, line 287 at r1 (raw file):
Previously, ujoni (Joni) wrote…
Print should be removed
Done.
There was a problem hiding this comment.
Reviewed 2 of 2 files at r2.
Reviewable status: 1 unresolved discussion, 1 of 1 LGTMs obtained (waiting on @ujoni)
There was a problem hiding this comment.
Reviewable status:
complete! all discussions resolved, 1 of 1 LGTMs obtained
…ged (#6385) Stores hash of the generated package json file in the main package json (inside node_modules, outside target folder) and updates it every time when the generated content is changed. TaskUpdatePackages modified status is changed only if the hash value is changed. It allows do not run npm install even if the generated package.json file is regenerated. Fixes #6151 (cherry picked from commit d887d09)
…ged (#6385) Stores hash of the generated package json file in the main package json (inside node_modules, outside target folder) and updates it every time when the generated content is changed. TaskUpdatePackages modified status is changed only if the hash value is changed. It allows do not run npm install even if the generated package.json file is regenerated. Fixes #6151 (cherry picked from commit d887d09)
mehdi-vaadin
moved this from Done - pending release
to Released
in OLD Vaadin Flow ongoing work (Vaadin 10+)
Stores hash of the generated package json file in the main package json
(inside node_modules, outside target folder) and updates it every time
when the generated content is changed. TaskUpdatePackages modified
status is changed only if the hash value is changed. It allows do not
run npm install even if the generated package.json file is regenerated.
Fixes #6151
This change is