Skip to content

Commit

Permalink
chore: Upgrade to Node 18 (#14635)
Browse files Browse the repository at this point in the history 
Node 18 becomes LTS before Vaadin 23.3 is relased

Fix installation of node for unix systems.
NodeJS 18 is more strict about where the contents are located than old versions.

Fixes #14636, #14661
  • Loading branch information
@Artur-
Artur- authored Sep 28, 2022
1 parent 4509a3f commit 672d29c
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 23 deletions.
10 changes: 7 additions & 3 deletions flow-server/src/main/java/com/vaadin/flow/server/frontend/FrontendTools.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,17 +66,21 @@ public class FrontendTools {
* the installed version is older than {@link #SUPPORTED_NODE_VERSION}, i.e.
* {@value #SUPPORTED_NODE_MAJOR_VERSION}.{@value #SUPPORTED_NODE_MINOR_VERSION}.
*/
public static final String DEFAULT_NODE_VERSION = "v16.17.1";
public static final String DEFAULT_NODE_VERSION = "v18.9.1";
/**
* This is the version shipped with the default Node version.
*/
public static final String DEFAULT_NPM_VERSION = "8.15.0";
public static final String DEFAULT_NPM_VERSION = "8.19.1";

public static final String DEFAULT_PNPM_VERSION = "5.18.10";

public static final String INSTALL_NODE_LOCALLY = "%n $ mvn com.github.eirslett:frontend-maven-plugin:1.10.0:install-node-and-npm "
+ "-DnodeVersion=\"" + DEFAULT_NODE_VERSION + "\" ";

public static final String NPM_BIN_PATH = FrontendUtils.isWindows()
? "node/node_modules/npm/bin/"
: "node/lib/node_modules/npm/bin/";

private static final String MSG_PREFIX = "%n%n======================================================================================================";
private static final String MSG_SUFFIX = "%n======================================================================================================%n";

Expand Down Expand Up @@ -1061,7 +1065,7 @@ private List<String> getNpmScriptCommand(String dir, String scriptName) {
// If `node` is not found in PATH, `node/node_modules/npm/bin/npm` will
// not work because it's a shell or windows script that looks for node
// and will fail. Thus we look for the `npm-cli` node script instead
File file = new File(dir, "node/node_modules/npm/bin/" + scriptName);
File file = new File(dir, NPM_BIN_PATH + scriptName);
List<String> returnCommand = new ArrayList<>();
if (file.canRead()) {
// We return a two element list with node binary and npm-cli script
Expand Down
52 changes: 47 additions & 5 deletions flow-server/src/main/java/com/vaadin/flow/server/frontend/installer/NodeInstaller.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ private void installNodeUnix(InstallData data)
+ nodeBinary);
}

File destinationDirectory = getNodeInstallDirectory();
File destinationDirectory = getNodeBinaryInstallDirectory();

File destination = new File(destinationDirectory,
data.getNodeExecutable());
Expand All @@ -302,8 +302,10 @@ private void installNodeUnix(InstallData data)
+ destination + " executable.");
}

createSymbolicLinkToBinary(destination, data.getNodeExecutable());

if (npmProvided()) {
extractUnixNpm(data, destinationDirectory);
extractUnixNpm(data, getNodeInstallDirectory());
}

deleteTempDirectory(data.getTmpDirectory());
Expand All @@ -315,7 +317,8 @@ private void extractUnixNpm(InstallData data, File destinationDirectory)
File tmpNodeModulesDir = new File(data.getTmpDirectory(),
data.getNodeFilename() + File.separator + "lib" + File.separator
+ FrontendUtils.NODE_MODULES);
File nodeModulesDirectory = new File(destinationDirectory,
File nodeModulesDirectory = new File(
destinationDirectory + File.separator + "lib",
FrontendUtils.NODE_MODULES);
File npmDirectory = new File(nodeModulesDirectory, "npm");

Expand All @@ -324,6 +327,13 @@ private void extractUnixNpm(InstallData data, File destinationDirectory)
if (nodeModulesDirectory.exists()) {
FileUtils.deleteDirectory(nodeModulesDirectory);
}
// delete old/windows type node_modules so it is not messing
// up the installation
final File oldNodeModulesDirectory = new File(destinationDirectory
+ File.separator + FrontendUtils.NODE_MODULES);
if (oldNodeModulesDirectory.exists()) {
FileUtils.deleteDirectory(oldNodeModulesDirectory);
}

FileUtils.copyDirectory(tmpNodeModulesDir, nodeModulesDirectory);
// create a copy of the npm scripts next to the node executable
Expand Down Expand Up @@ -398,11 +408,15 @@ private void copyNodeBinaryToDestination(File nodeBinary, File destination)
}

public String getInstallDirectory() {
return new File(installDirectory, INSTALL_PATH).getPath();
return getInstallDirectoryFile().getPath();
}

private File getInstallDirectoryFile() {
return new File(installDirectory, INSTALL_PATH);
}

private File getNodeInstallDirectory() {
File nodeInstallDirectory = new File(installDirectory, INSTALL_PATH);
File nodeInstallDirectory = getInstallDirectoryFile();
if (!nodeInstallDirectory.exists()) {
getLogger().debug("Creating install directory {}",
nodeInstallDirectory);
Expand All @@ -414,6 +428,34 @@ private File getNodeInstallDirectory() {
return nodeInstallDirectory;
}

private File getNodeBinaryInstallDirectory() {
File nodeInstallDirectory = new File(getInstallDirectoryFile(), "bin");
if (!nodeInstallDirectory.exists()) {
getLogger().debug("Creating install directory {}",
nodeInstallDirectory);
boolean success = nodeInstallDirectory.mkdirs();
if (!success) {
getLogger().debug("Failed to create install directory");
}
}
return nodeInstallDirectory;
}

private void createSymbolicLinkToBinary(File destination,
String nodeExecutable) throws InstallationException, IOException {
final File symLink = new File(getInstallDirectory(), nodeExecutable);
if (symLink.exists()) {
FileUtils.delete(symLink);
}
try {
Files.createSymbolicLink(symLink.toPath(), destination.toPath());
} catch (IOException e) {
throw new InstallationException(String.format(
"Could not install Node: Was not allowed to create symbolic link %s for %s",
symLink, destination), e);
}
}

private void deleteTempDirectory(File tmpDirectory) throws IOException {
if (tmpDirectory != null && tmpDirectory.exists()) {
getLogger().debug("Deleting temporary directory {}", tmpDirectory);
Expand Down
32 changes: 20 additions & 12 deletions flow-server/src/test/java/com/vaadin/flow/server/frontend/FrontendToolsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package com.vaadin.flow.server.frontend;

import static com.vaadin.flow.server.frontend.FrontendTools.NPM_BIN_PATH;
import static com.vaadin.flow.testutil.FrontendStubs.createStubNode;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.not;
Expand Down Expand Up @@ -73,9 +74,9 @@ public class FrontendToolsTest {
? "node\\node.exe"
: "node/node";

public static final String NPM_CLI_STRING = Stream
.of("node", "node_modules", "npm", "bin", "npm-cli.js")
.collect(Collectors.joining(File.separator));
public static final String NPM_CLI_STRING = FrontendUtils.isWindows()
? "node\\node_modules\\npm\\bin\\npm-cli.js"
: "node/lib/node_modules/npm/bin/npm-cli.js";

private static final String OLD_PNPM_VERSION = "4.5.0";

Expand Down Expand Up @@ -130,8 +131,13 @@ public void installNode_NodeIsInstalledToTargetDirectory()
npmVersionCommand.add("--version");
FrontendVersion npm = FrontendUtils.getVersion("npm",
npmVersionCommand);
Assert.assertEquals(FrontendTools.DEFAULT_NPM_VERSION,
npm.getFullVersion());
final FrontendVersion npmDefault = new FrontendVersion(
FrontendTools.DEFAULT_NPM_VERSION);

Assert.assertEquals("Major version should match",
npmDefault.getMajorVersion(), npm.getMajorVersion());
Assert.assertEquals("Minor version should match",
npmDefault.getMinorVersion(), npm.getMinorVersion());
}

@Test
Expand Down Expand Up @@ -339,27 +345,28 @@ public void installNodeFromFileSystem_NodeIsInstalledToTargetDirectory()
String nodeExecutable = installNodeToTempFolder();
Assert.assertNotNull(nodeExecutable);

String npmInstallPath = NPM_BIN_PATH + "npm";

Assert.assertTrue("npm should have been copied to node_modules",
new File(vaadinHomeDir, "node/node_modules/npm/bin/npm")
.exists());
new File(vaadinHomeDir, npmInstallPath).exists());
}

@Test
public void installNodeFromFileSystem_ForceAlternativeNodeExecutableInstallsToTargetDirectory()
throws Exception {
Assert.assertFalse("npm should not yet be present",
new File(vaadinHomeDir, "node/node_modules/npm/bin/npm")
.exists());
new File(vaadinHomeDir, NPM_BIN_PATH + "npm").exists());

settings.setNodeDownloadRoot(new File(baseDir).toURI());
settings.setNodeVersion("v12.10.0");
tools = new FrontendTools(settings);
prepareNodeDownloadableZipAt(baseDir, "v12.10.0");
tools.forceAlternativeNodeExecutable();

String npmInstallPath = NPM_BIN_PATH + "npm";

Assert.assertTrue("npm should have been copied to node_modules",
new File(vaadinHomeDir, "node/node_modules/npm/bin/npm")
.exists());
new File(vaadinHomeDir, npmInstallPath).exists());
}

@Test
Expand Down Expand Up @@ -893,7 +900,8 @@ private void assertFaultyNpmVersion(FrontendVersion version) {
}

private void createFakePnpm(String defaultPnpmVersion) throws Exception {
File npxJs = new File(baseDir, "node/node_modules/npm/bin/npx-cli.js");
final String npxPath = NPM_BIN_PATH + "npx-cli.js";
File npxJs = new File(baseDir, npxPath);
FileUtils.forceMkdir(npxJs.getParentFile());

FileWriter fileWriter = new FileWriter(npxJs);
Expand Down
18 changes: 16 additions & 2 deletions flow-server/src/test/java/com/vaadin/flow/server/frontend/installer/NodeInstallerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,21 @@ public void installNodeFromFileSystem_NodeIsInstalledToTargetDirectory()

// add a file to node/node_modules_npm that should be cleaned out
File nodeDirectory = new File(targetDir, "node");
File nodeModulesDirectory = new File(nodeDirectory, "node_modules");
String nodeModulesPath = platform.isWindows() ? "node_modules"
: "lib/node_modules";
File nodeModulesDirectory = new File(nodeDirectory, nodeModulesPath);
File npmDirectory = new File(nodeModulesDirectory, "npm");
File garbage = new File(npmDirectory, "garbage");
FileUtils.forceMkdir(npmDirectory);
Assert.assertTrue("garbage file should be created",
garbage.createNewFile());

File oldNpm = new File(nodeDirectory, "node_modules/npm");
File oldGarbage = new File(oldNpm, "oldGarbage");
FileUtils.forceMkdir(oldNpm);
Assert.assertTrue("oldGarbage file should be created",
oldGarbage.createNewFile());

NodeInstaller nodeInstaller = new NodeInstaller(targetDir,
Collections.emptyList())
.setNodeVersion(FrontendTools.DEFAULT_NODE_VERSION)
Expand All @@ -112,9 +120,15 @@ public void installNodeFromFileSystem_NodeIsInstalledToTargetDirectory()

Assert.assertTrue("npm should have been copied to node_modules",
new File(targetDir, "node/" + nodeExec).exists());
String npmInstallPath = platform.isWindows()
? "node/node_modules/npm/bin/npm"
: "node/lib/node_modules/npm/bin/npm";
Assert.assertTrue("npm should have been copied to node_modules",
new File(targetDir, "node/node_modules/npm/bin/npm").exists());
new File(targetDir, npmInstallPath).exists());
Assert.assertFalse("old npm files should have been removed",
garbage.exists());
Assert.assertFalse(
"old style node_modules files should have been removed",
oldGarbage.exists());
}
}
4 changes: 3 additions & 1 deletion flow-test-generic/src/main/java/com/vaadin/flow/testutil/FrontendStubs.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ public class FrontendStubs {
public static final String VITE_SERVER = "node_modules/vite/bin/vite.js";
public static final String VITE_TEST_OUT_FILE = "vite-out.test";

private static final String NPM_BIN_PATH = "node/node_modules/npm/bin";
private static final String NPM_BIN_PATH = System.getProperty("os.name")
.startsWith("Windows") ? "node/node_modules/npm/bin/"
: "node/lib/node_modules/npm/bin/";
private static final String NPM_CACHE_PATH_STUB = "cache";

/**
Expand Down

0 comments on commit 672d29c

Please sign in to comment.