fix: remove home node_modules before install (#13172) (#13178)

Files left by previous node+npm installation caused
corrupted installation when unpacking on top of an
existing installation. Fixed by removing
~/.vaadin/node/node_modules prior to
installation of newer version.

Also removes overrides management of 
devDependencies.

Co-authored-by: Mikael Grankvist <mikael.grankvist@vaadin.com>
Co-authored-by: Johannes Eriksson <joheriks@vaadin.com>

flow-plugins/flow-gradle-plugin/src/functionalTest/kotlin/com/vaadin/gradle/AbstractGradleTest.kt

@@ -27,7 +27,7 @@ import java.io.File
  */
 abstract class AbstractGradleTest {
 
-    val flowVersion = System.getenv("vaadin.version").takeUnless { it.isNullOrEmpty() } ?: "10.0-SNAPSHOT"
+    val flowVersion = System.getenv("vaadin.version").takeUnless { it.isNullOrEmpty() } ?: "23.1-SNAPSHOT"
 
     /**
      * The testing Gradle project. Automatically deleted after every test.

flow-plugins/flow-gradle-plugin/src/functionalTest/kotlin/com/vaadin/gradle/MiscSingleModuleTest.kt

@@ -221,13 +221,12 @@ class MiscSingleModuleTest : AbstractGradleTest() {
     fun testSpringProjectProductionMode() {
 
         val springBootVersion = "2.2.4.RELEASE"
-        val springVersion = "17.0-SNAPSHOT"
 
         testProject.buildFile.writeText(
             """
             plugins {
                 id 'org.springframework.boot' version '$springBootVersion'
-                id 'io.spring.dependency-management' version '1.0.9.RELEASE'
+                id 'io.spring.dependency-management' version '1.0.11.RELEASE'
                 id 'java'
                 id("com.vaadin")
             }
@@ -247,7 +246,7 @@ class MiscSingleModuleTest : AbstractGradleTest() {
             
             dependencies {
                 implementation('com.vaadin:flow:$flowVersion')
-                implementation('com.vaadin:vaadin-spring:$springVersion')
+                implementation('com.vaadin:vaadin-spring:$flowVersion')
                 implementation('org.springframework.boot:spring-boot-starter-web:$springBootVersion')
                 developmentOnly 'org.springframework.boot:spring-boot-devtools'
                 testImplementation('org.springframework.boot:spring-boot-starter-test') {

flow-plugins/flow-gradle-plugin/src/main/kotlin/com/vaadin/gradle/VaadinBuildFrontendTask.kt

@@ -68,14 +68,14 @@ public open class VaadinBuildFrontendTask : DefaultTask() {
         val tokenFile = BuildFrontendUtil.getTokenFile(adapter)
         check(tokenFile.exists()) { "token file $tokenFile doesn't exist!" }
 
-        BuildFrontendUtil.updateBuildFile(adapter)
-
         BuildFrontendUtil.runNodeUpdater(adapter)
 
         if (adapter.generateBundle()) {
             BuildFrontendUtil.runFrontendBuild(adapter)
         } else {
             logger.info("Not running webpack since generateBundle is false")
         }
+
+        BuildFrontendUtil.updateBuildFile(adapter)
     }
 }

flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskUpdatePackages.java

@@ -139,12 +139,9 @@ boolean lockVersionForNpm(JsonObject packageJson, String versionsPath)
 
         JsonObject overridesSection = getOverridesSection(packageJson);
         final JsonObject dependencies = packageJson.getObject(DEPENDENCIES);
-        final JsonObject devDependencies = packageJson
-                .getObject(DEV_DEPENDENCIES);
         for (String dependency : versionsJson.keys()) {
             if (!overridesSection.hasKey(dependency)
-                    && (dependencies.hasKey(dependency)
-                            || devDependencies.hasKey(dependency))
+                    && dependencies.hasKey(dependency)
                     && !isInternalPseudoDependency(
                             versionsJson.getString(dependency))) {
                 overridesSection.put(dependency, "$" + dependency);
@@ -153,7 +150,6 @@ boolean lockVersionForNpm(JsonObject packageJson, String versionsPath)
         }
         for (String dependency : overridesSection.keys()) {
             if (!dependencies.hasKey(dependency)
-                    && !devDependencies.hasKey(dependency)
                     && overridesSection.getString(dependency).startsWith("$")) {
                 overridesSection.remove(dependency);
                 versionLockingUpdated = true;

flow-server/src/main/java/com/vaadin/flow/server/frontend/installer/NodeInstaller.java

@@ -316,6 +316,13 @@ private void extractUnixNpm(InstallData data, File destinationDirectory)
         File nodeModulesDirectory = new File(destinationDirectory,
                 FrontendUtils.NODE_MODULES);
         File npmDirectory = new File(nodeModulesDirectory, "npm");
+
+        // delete old node_modules directory to not end up with corrupted
+        // combination of two npm versions in node_modules/npm during upgrade
+        if (nodeModulesDirectory.exists()) {
+            FileUtils.deleteDirectory(nodeModulesDirectory);
+        }
+
         FileUtils.copyDirectory(tmpNodeModulesDir, nodeModulesDirectory);
         // create a copy of the npm scripts next to the node executable
         for (String script : Arrays.asList("npm", "npm.cmd")) {
@@ -357,6 +364,12 @@ private void installNodeWindows(InstallData data)
                             + FrontendUtils.NODE_MODULES);
             File nodeModulesDirectory = new File(destinationDirectory,
                     FrontendUtils.NODE_MODULES);
+            // delete old node_modules directory to not end up with corrupted
+            // combination of two npm versions in node_modules/npm during
+            // upgrade
+            if (nodeModulesDirectory.exists()) {
+                FileUtils.deleteDirectory(nodeModulesDirectory);
+            }
             FileUtils.copyDirectory(tmpNodeModulesDir, nodeModulesDirectory);
         }
         deleteTempDirectory(data.getTmpDirectory());

flow-server/src/test/java/com/vaadin/flow/server/frontend/installer/NodeInstallerTest.java

@@ -12,6 +12,7 @@
 import org.apache.commons.compress.archivers.ArchiveOutputStream;
 import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream;
 import org.apache.commons.compress.compressors.gzip.GzipCompressorOutputStream;
+import org.apache.commons.io.FileUtils;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Rule;
@@ -89,6 +90,15 @@ 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");
+        File npmDirectory = new File(nodeModulesDirectory, "npm");
+        File garbage = new File(npmDirectory, "garbage");
+        FileUtils.forceMkdir(npmDirectory);
+        Assert.assertTrue("garbage file should be created",
+                garbage.createNewFile());
+
         NodeInstaller nodeInstaller = new NodeInstaller(targetDir,
                 Collections.emptyList())
                         .setNodeVersion(FrontendTools.DEFAULT_NODE_VERSION)
@@ -105,5 +115,7 @@ public void installNodeFromFileSystem_NodeIsInstalledToTargetDirectory()
                 new File(targetDir, "node/" + nodeExec).exists());
         Assert.assertTrue("npm should have been copied to node_modules",
                 new File(targetDir, "node/node_modules/npm/bin/npm").exists());
+        Assert.assertFalse("old npm files should have been removed",
+                garbage.exists());
     }
 }