Skip to content

Commit 39c84f2

Browse files
authored
speedup npm install step for npm based formatters (#1590 fixes #1480 and #1582)
2 parents 6716228 + 8ebb896 commit 39c84f2

40 files changed

+1966
-172
lines changed

CHANGES.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,12 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
1313
### Added
1414
* `gradlew equoIde` opens a repeatable clean Spotless dev environment. ([#1523](https://github.com/diffplug/spotless/pull/1523))
1515
* `cleanthat` added `includeDraft` option, to include draft mutators from composite mutators. ([#1574](https://github.com/diffplug/spotless/pull/1574))
16+
* `npm`-based formatters now support caching of `node_modules` directory ([#1590](https://github.com/diffplug/spotless/pull/1590))
1617
### Fixed
1718
* `JacksonJsonFormatterFunc` handles json files with an Array as root. ([#1585](https://github.com/diffplug/spotless/pull/1585))
1819
### Changes
1920
* Bump default `cleanthat` version to latest `2.1` -> `2.6` ([#1569](https://github.com/diffplug/spotless/pull/1569) and [#1574](https://github.com/diffplug/spotless/pull/1574))
21+
* Reduce logging-noise created by `npm`-based formatters ([#1590](https://github.com/diffplug/spotless/pull/1590) fixes [#1582](https://github.com/diffplug/spotless/issues/1582))
2022

2123
## [2.35.0] - 2023-02-10
2224
### Added

lib/build.gradle

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ tasks.named("check").configure {
5656

5757
dependencies {
5858
compileOnly 'org.slf4j:slf4j-api:2.0.0'
59+
testCommonImplementation 'org.slf4j:slf4j-api:2.0.0'
60+
5961
// zero runtime reqs is a hard requirements for spotless-lib
6062
// if you need a dep, put it in lib-extra
6163
testCommonImplementation "org.junit.jupiter:junit-jupiter:$VER_JUNIT"

lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
*/
1616
package com.diffplug.spotless.npm;
1717

18-
import static com.diffplug.spotless.LazyArgLogger.lazy;
1918
import static java.util.Objects.requireNonNull;
2019

2120
import java.io.File;
@@ -71,13 +70,13 @@ public static Map<String, String> defaultDevDependenciesWithEslint(String versio
7170
return Collections.singletonMap("eslint", version);
7271
}
7372

74-
public static FormatterStep create(Map<String, String> devDependencies, Provisioner provisioner, File projectDir, File buildDir, NpmPathResolver npmPathResolver, EslintConfig eslintConfig) {
73+
public static FormatterStep create(Map<String, String> devDependencies, Provisioner provisioner, File projectDir, File buildDir, File cacheDir, NpmPathResolver npmPathResolver, EslintConfig eslintConfig) {
7574
requireNonNull(devDependencies);
7675
requireNonNull(provisioner);
7776
requireNonNull(projectDir);
7877
requireNonNull(buildDir);
7978
return FormatterStep.createLazy(NAME,
80-
() -> new State(NAME, devDependencies, projectDir, buildDir, npmPathResolver, eslintConfig),
79+
() -> new State(NAME, devDependencies, projectDir, buildDir, cacheDir, npmPathResolver, eslintConfig),
8180
State::createFormatterFunc);
8281
}
8382

@@ -89,20 +88,20 @@ private static class State extends NpmFormatterStepStateBase implements Serializ
8988
@SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED")
9089
private transient EslintConfig eslintConfigInUse;
9190

92-
State(String stepName, Map<String, String> devDependencies, File projectDir, File buildDir, NpmPathResolver npmPathResolver, EslintConfig eslintConfig) throws IOException {
91+
State(String stepName, Map<String, String> devDependencies, File projectDir, File buildDir, File cacheDir, NpmPathResolver npmPathResolver, EslintConfig eslintConfig) throws IOException {
9392
super(stepName,
9493
new NpmConfig(
9594
replaceDevDependencies(
9695
NpmResourceHelper.readUtf8StringFromClasspath(EslintFormatterStep.class, "/com/diffplug/spotless/npm/eslint-package.json"),
9796
new TreeMap<>(devDependencies)),
98-
"eslint",
9997
NpmResourceHelper.readUtf8StringFromClasspath(EslintFormatterStep.class,
10098
"/com/diffplug/spotless/npm/common-serve.js",
10199
"/com/diffplug/spotless/npm/eslint-serve.js"),
102100
npmPathResolver.resolveNpmrcContent()),
103101
new NpmFormatterStepLocations(
104102
projectDir,
105103
buildDir,
104+
cacheDir,
106105
npmPathResolver::resolveNpmExecutable,
107106
npmPathResolver::resolveNodeExecutable));
108107
this.origEslintConfig = requireNonNull(eslintConfig.verify());
@@ -116,7 +115,7 @@ protected void prepareNodeServerLayout() throws IOException {
116115
// If any config files are provided, we need to make sure they are at the same location as the node modules
117116
// as eslint will try to resolve plugin/config names relatively to the config file location and some
118117
// eslint configs contain relative paths to additional config files (such as tsconfig.json e.g.)
119-
logger.info("Copying config file <{}> to <{}> and using the copy", origEslintConfig.getEslintConfigPath(), nodeServerLayout.nodeModulesDir());
118+
logger.debug("Copying config file <{}> to <{}> and using the copy", origEslintConfig.getEslintConfigPath(), nodeServerLayout.nodeModulesDir());
120119
File configFileCopy = NpmResourceHelper.copyFileToDir(origEslintConfig.getEslintConfigPath(), nodeServerLayout.nodeModulesDir());
121120
this.eslintConfigInUse = this.origEslintConfig.withEslintConfigPath(configFileCopy).verify();
122121
}
@@ -162,8 +161,6 @@ public EslintFilePathPassingFormatterFunc(File projectDir, File nodeModulesDir,
162161

163162
@Override
164163
public String applyWithFile(String unix, File file) throws Exception {
165-
logger.info("formatting String '{}[...]' in file '{}'", lazy(() -> unix.substring(0, Math.min(50, unix.length()))), file);
166-
167164
Map<FormatOption, Object> eslintCallOptions = new HashMap<>();
168165
setConfigToCallOptions(eslintCallOptions);
169166
setFilePathToCallOptions(eslintCallOptions, file);
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
/*
2+
* Copyright 2023 DiffPlug
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.diffplug.spotless.npm;
17+
18+
import java.util.Objects;
19+
20+
import javax.annotation.Nonnull;
21+
22+
import org.slf4j.Logger;
23+
import org.slf4j.LoggerFactory;
24+
25+
public class NodeApp {
26+
27+
private static final Logger logger = LoggerFactory.getLogger(NodeApp.class);
28+
29+
private static final TimedLogger timedLogger = TimedLogger.forLogger(logger);
30+
31+
@Nonnull
32+
protected final NodeServerLayout nodeServerLayout;
33+
34+
@Nonnull
35+
protected final NpmConfig npmConfig;
36+
37+
@Nonnull
38+
protected final NpmProcessFactory npmProcessFactory;
39+
40+
@Nonnull
41+
protected final NpmFormatterStepLocations formatterStepLocations;
42+
43+
public NodeApp(@Nonnull NodeServerLayout nodeServerLayout, @Nonnull NpmConfig npmConfig, @Nonnull NpmFormatterStepLocations formatterStepLocations) {
44+
this.nodeServerLayout = Objects.requireNonNull(nodeServerLayout);
45+
this.npmConfig = Objects.requireNonNull(npmConfig);
46+
this.npmProcessFactory = processFactory(formatterStepLocations);
47+
this.formatterStepLocations = Objects.requireNonNull(formatterStepLocations);
48+
}
49+
50+
private static NpmProcessFactory processFactory(NpmFormatterStepLocations formatterStepLocations) {
51+
if (formatterStepLocations.cacheDir() != null) {
52+
logger.info("Caching npm install results in {}.", formatterStepLocations.cacheDir());
53+
return NodeModulesCachingNpmProcessFactory.create(formatterStepLocations.cacheDir());
54+
}
55+
logger.debug("Not caching npm install results.");
56+
return StandardNpmProcessFactory.INSTANCE;
57+
}
58+
59+
boolean needsNpmInstall() {
60+
return !this.nodeServerLayout.isNodeModulesPrepared();
61+
}
62+
63+
boolean needsPrepareNodeAppLayout() {
64+
return !this.nodeServerLayout.isLayoutPrepared();
65+
}
66+
67+
void prepareNodeAppLayout() {
68+
timedLogger.withInfo("Preparing {} for npm step {}.", this.nodeServerLayout, getClass().getName()).run(() -> {
69+
NpmResourceHelper.assertDirectoryExists(nodeServerLayout.nodeModulesDir());
70+
NpmResourceHelper.writeUtf8StringToFile(nodeServerLayout.packageJsonFile(), this.npmConfig.getPackageJsonContent());
71+
if (this.npmConfig.getServeScriptContent() != null) {
72+
NpmResourceHelper.writeUtf8StringToFile(nodeServerLayout.serveJsFile(), this.npmConfig.getServeScriptContent());
73+
} else {
74+
NpmResourceHelper.deleteFileIfExists(nodeServerLayout.serveJsFile());
75+
}
76+
if (this.npmConfig.getNpmrcContent() != null) {
77+
NpmResourceHelper.writeUtf8StringToFile(nodeServerLayout.npmrcFile(), this.npmConfig.getNpmrcContent());
78+
} else {
79+
NpmResourceHelper.deleteFileIfExists(nodeServerLayout.npmrcFile());
80+
}
81+
});
82+
}
83+
84+
void npmInstall() {
85+
timedLogger.withInfo("Installing npm dependencies for {} with {}.", this.nodeServerLayout, this.npmProcessFactory.describe())
86+
.run(() -> npmProcessFactory.createNpmInstallProcess(nodeServerLayout, formatterStepLocations).waitFor());
87+
}
88+
}
Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
/*
2+
* Copyright 2023 DiffPlug
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.diffplug.spotless.npm;
17+
18+
import java.io.File;
19+
import java.util.List;
20+
import java.util.Objects;
21+
22+
import javax.annotation.Nonnull;
23+
24+
import org.slf4j.Logger;
25+
import org.slf4j.LoggerFactory;
26+
27+
import com.diffplug.spotless.ProcessRunner.Result;
28+
29+
public class NodeModulesCachingNpmProcessFactory implements NpmProcessFactory {
30+
31+
private static final Logger logger = LoggerFactory.getLogger(NodeModulesCachingNpmProcessFactory.class);
32+
33+
private static final TimedLogger timedLogger = TimedLogger.forLogger(logger);
34+
35+
private final File cacheDir;
36+
37+
private final ShadowCopy shadowCopy;
38+
39+
private NodeModulesCachingNpmProcessFactory(@Nonnull File cacheDir) {
40+
this.cacheDir = Objects.requireNonNull(cacheDir);
41+
assertDir(cacheDir);
42+
this.shadowCopy = new ShadowCopy(cacheDir);
43+
}
44+
45+
private void assertDir(File cacheDir) {
46+
if (cacheDir.exists() && !cacheDir.isDirectory()) {
47+
throw new IllegalArgumentException("Cache dir must be a directory");
48+
}
49+
if (!cacheDir.exists()) {
50+
if (!cacheDir.mkdirs()) {
51+
throw new IllegalArgumentException("Cache dir could not be created.");
52+
}
53+
}
54+
}
55+
56+
public static NodeModulesCachingNpmProcessFactory create(@Nonnull File cacheDir) {
57+
return new NodeModulesCachingNpmProcessFactory(cacheDir);
58+
}
59+
60+
@Override
61+
public NpmProcess createNpmInstallProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations) {
62+
NpmProcess actualNpmInstallProcess = StandardNpmProcessFactory.INSTANCE.createNpmInstallProcess(nodeServerLayout, formatterStepLocations);
63+
return new CachingNmpInstall(actualNpmInstallProcess, nodeServerLayout);
64+
}
65+
66+
@Override
67+
public NpmLongRunningProcess createNpmServeProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations) {
68+
return StandardNpmProcessFactory.INSTANCE.createNpmServeProcess(nodeServerLayout, formatterStepLocations);
69+
}
70+
71+
private class CachingNmpInstall implements NpmProcess {
72+
73+
private final NpmProcess actualNpmInstallProcess;
74+
private final NodeServerLayout nodeServerLayout;
75+
76+
public CachingNmpInstall(NpmProcess actualNpmInstallProcess, NodeServerLayout nodeServerLayout) {
77+
this.actualNpmInstallProcess = actualNpmInstallProcess;
78+
this.nodeServerLayout = nodeServerLayout;
79+
}
80+
81+
@Override
82+
public Result waitFor() {
83+
String entryName = entryName();
84+
if (shadowCopy.entryExists(entryName, NodeServerLayout.NODE_MODULES)) {
85+
timedLogger.withInfo("Using cached node_modules for {} from {}", entryName, cacheDir)
86+
.run(() -> shadowCopy.copyEntryInto(entryName(), NodeServerLayout.NODE_MODULES, nodeServerLayout.nodeModulesDir()));
87+
return new CachedResult();
88+
} else {
89+
Result result = timedLogger.withInfo("calling actual npm install {}", actualNpmInstallProcess.describe())
90+
.call(actualNpmInstallProcess::waitFor);
91+
assert result.exitCode() == 0;
92+
storeShadowCopy(entryName);
93+
return result;
94+
}
95+
}
96+
97+
private void storeShadowCopy(String entryName) {
98+
timedLogger.withInfo("Caching node_modules for {} in {}", entryName, cacheDir)
99+
.run(() -> shadowCopy.addEntry(entryName(), new File(nodeServerLayout.nodeModulesDir(), NodeServerLayout.NODE_MODULES)));
100+
}
101+
102+
private String entryName() {
103+
return nodeServerLayout.nodeModulesDir().getName();
104+
}
105+
106+
@Override
107+
public String describe() {
108+
return String.format("Wrapper around [%s] to cache node_modules in [%s]", actualNpmInstallProcess.describe(), cacheDir.getAbsolutePath());
109+
}
110+
}
111+
112+
private class CachedResult extends Result {
113+
114+
public CachedResult() {
115+
super(List.of("(from cache dir " + cacheDir + ")"), 0, new byte[0], new byte[0]);
116+
}
117+
}
118+
}
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/*
2+
* Copyright 2023 DiffPlug
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.diffplug.spotless.npm;
17+
18+
import javax.annotation.Nonnull;
19+
20+
import org.slf4j.Logger;
21+
import org.slf4j.LoggerFactory;
22+
23+
import com.diffplug.spotless.ProcessRunner;
24+
25+
public class NodeServeApp extends NodeApp {
26+
27+
private static final Logger logger = LoggerFactory.getLogger(NodeApp.class);
28+
29+
private static final TimedLogger timedLogger = TimedLogger.forLogger(logger);
30+
31+
public NodeServeApp(@Nonnull NodeServerLayout nodeServerLayout, @Nonnull NpmConfig npmConfig, @Nonnull NpmFormatterStepLocations formatterStepLocations) {
32+
super(nodeServerLayout, npmConfig, formatterStepLocations);
33+
}
34+
35+
ProcessRunner.LongRunningProcess startNpmServeProcess() {
36+
return timedLogger.withInfo("Starting npm based server in {} with {}.", this.nodeServerLayout.nodeModulesDir(), this.npmProcessFactory.describe())
37+
.call(() -> npmProcessFactory.createNpmServeProcess(nodeServerLayout, formatterStepLocations).start());
38+
}
39+
40+
}

lib/src/main/java/com/diffplug/spotless/npm/NodeServerLayout.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
class NodeServerLayout {
2828

2929
private static final Pattern PACKAGE_JSON_NAME_PATTERN = Pattern.compile("\"name\"\\s*:\\s*\"([^\"]+)\"");
30+
static final String NODE_MODULES = "node_modules";
3031

3132
private final File nodeModulesDir;
3233
private final File packageJsonFile;
@@ -55,7 +56,6 @@ private static String nodeModulesDirName(String packageJsonContent) {
5556
}
5657

5758
File nodeModulesDir() {
58-
5959
return nodeModulesDir;
6060
}
6161

@@ -89,7 +89,7 @@ public boolean isLayoutPrepared() {
8989
}
9090

9191
public boolean isNodeModulesPrepared() {
92-
Path nodeModulesInstallDirPath = new File(nodeModulesDir(), "node_modules").toPath();
92+
Path nodeModulesInstallDirPath = new File(nodeModulesDir(), NODE_MODULES).toPath();
9393
if (!Files.isDirectory(nodeModulesInstallDirPath)) {
9494
return false;
9595
}

0 commit comments

Comments
 (0)