Skip to content

Commit

Permalink
fix: Do not override user pinned versions with pnpm (#10635)
Browse files Browse the repository at this point in the history
Any user defined (package.json / NpmPackage-annotation) versions will not be overridden by based on the platform versions (versions.json file).
This means that the users will have to remember to also remove their pins after those are not needed anymore or they will stay on using old versions, which is the same as with Maven overrides.

Fixes #9793
  • Loading branch information
caalador authored and vaadin-bot committed Apr 16, 2021
1 parent 6cdc843 commit 9ae0550
Show file tree
Hide file tree
Showing 6 changed files with 153 additions and 17 deletions.
Expand Up @@ -418,7 +418,7 @@ private void runNpmInstall() throws ExecutionFailedException {
String stdoutLine;
while ((stdoutLine = reader.readLine()) != null) {
packageUpdater.log().debug(stdoutLine);
toolOutput.append(stdoutLine);
toolOutput.append(stdoutLine).append(System.lineSeparator());
}
}

Expand Down
Expand Up @@ -15,6 +15,8 @@
*/
package com.vaadin.flow.server.frontend;

import org.slf4j.LoggerFactory;

import elemental.json.Json;
import elemental.json.JsonObject;

Expand All @@ -29,12 +31,16 @@
class VersionsJsonFilter {

private final JsonObject userManagedDependencies;
private final JsonObject vaadinVersions;

private final String dependenciesKey;

private final String OLDER_VERSION_WARNING = "Using user pinned version '{}' of '{}' which is older than the current platform version '{}'";

VersionsJsonFilter(JsonObject packageJson, String dependenciesKey) {
this.dependenciesKey = dependenciesKey;
userManagedDependencies = collectUserManagedDependencies(packageJson);
vaadinVersions = collectFrameworkVersions(packageJson);
}

/**
Expand All @@ -46,15 +52,41 @@ class VersionsJsonFilter {
JsonObject getFilteredVersions(JsonObject versions) {
JsonObject json = Json.createObject();
for (String key : versions.keys()) {
if (!userManagedDependencies.hasKey(key)) {
json.put(key, versions.getString(key));
} else {
if (userManagedDependencies.hasKey(key)) {
if (isNewer(versions.getString(key),
userManagedDependencies.getString(key))) {
LoggerFactory.getLogger("Versions").warn(
OLDER_VERSION_WARNING,
userManagedDependencies.getString(key), key,
versions.getString(key));
}
json.put(key, userManagedDependencies.getString(key));
} else if (vaadinVersions.hasKey(key) && isNewer(
vaadinVersions.getString(key), versions.getString(key))) {
json.put(key, vaadinVersions.getString(key));
} else {
json.put(key, versions.getString(key));
}
}
return json;
}

/**
* Check if version1 is newer than version2.
*
* @param version1
* version string to see if newer
* @param version2
* version string to compare against
* @return true if version1 > version2
*/
private boolean isNewer(String version1, String version2) {
final FrontendVersion frontendVersion = new FrontendVersion(version1);
final FrontendVersion frontendVersion2 = new FrontendVersion(version2);

return frontendVersion.isNewerThan(frontendVersion2);
}

/**
* Collect all dependencies that the user has changed that do not match the
* flow managed dependency versions.
Expand All @@ -65,14 +97,7 @@ JsonObject getFilteredVersions(JsonObject versions) {
*/
private JsonObject collectUserManagedDependencies(JsonObject packageJson) {
JsonObject json = Json.createObject();
JsonObject vaadinDep;
if (packageJson.hasKey(VAADIN_DEP_KEY) && packageJson
.getObject(VAADIN_DEP_KEY).hasKey(dependenciesKey)) {
vaadinDep = packageJson.getObject(VAADIN_DEP_KEY)
.getObject(dependenciesKey);
} else {
vaadinDep = Json.createObject();
}
JsonObject vaadinDep = collectFrameworkVersions(packageJson);

if (packageJson.hasKey(dependenciesKey)) {
JsonObject dependencies = packageJson.getObject(dependenciesKey);
Expand All @@ -99,4 +124,20 @@ private boolean isUserChanged(String key, JsonObject vaadinDep,
// User changed if not in vaadin dependency
return true;
}

/**
* Get the Vaadin dependency.
*
* @param packageJson
* main package.json
* @return Vaadin dependencies or empty object
*/
private JsonObject collectFrameworkVersions(JsonObject packageJson) {
if (packageJson.hasKey(VAADIN_DEP_KEY) && packageJson
.getObject(VAADIN_DEP_KEY).hasKey(dependenciesKey)) {
return packageJson.getObject(VAADIN_DEP_KEY)
.getObject(dependenciesKey);
}
return Json.createObject();
}
}
Expand Up @@ -518,6 +518,94 @@ public void generateVersionsJson_userHasNoCustomVersions_platformIsMergedWithDev
object.getString("@vaadin/vaadin-notification"));
}

@Test
public void runPnpmInstall_userVersionNewerThanPinned_installedOverlayVersionIsNotSpecifiedByPlatform()
throws IOException, ExecutionFailedException {
File packageJson = new File(getNodeUpdater().npmFolder, PACKAGE_JSON);
packageJson.createNewFile();

// Write package json file
final String customOverlayVersion = "3.3.0";
// @formatter:off
final String packageJsonContent =
"{"
+ "\"dependencies\": {"
+ "\"@vaadin/vaadin-dialog\": \"2.3.0\","
+ "\"@vaadin/vaadin-overlay\": \"" + customOverlayVersion + "\""
+ "}"
+ "}";
// @formatter:on
FileUtils.write(packageJson, packageJsonContent,
StandardCharsets.UTF_8);

final VersionsJsonFilter versionsJsonFilter = new VersionsJsonFilter(
Json.parse(packageJsonContent), NodeUpdater.DEPENDENCIES);
// Platform defines a pinned version
TaskRunNpmInstall task = createTask(
versionsJsonFilter.getFilteredVersions(
Json.parse("{ \"@vaadin/vaadin-overlay\":\""
+ PINNED_VERSION + "\"}"))
.toJson());
task.execute();

File overlayPackageJson = new File(getNodeUpdater().nodeModulesFolder,
"@vaadin/vaadin-overlay/package.json");

// The resulting version should be the one specified by the user
JsonObject overlayPackage = Json.parse(FileUtils
.readFileToString(overlayPackageJson, StandardCharsets.UTF_8));
Assert.assertEquals(customOverlayVersion,
overlayPackage.getString("version"));
}

@Test
public void runPnpmInstall_frameworkCollectedVersionNewerThanPinned_installedOverlayVersionIsNotSpecifiedByPlatform()
throws IOException, ExecutionFailedException {
File packageJson = new File(getNodeUpdater().npmFolder, PACKAGE_JSON);
packageJson.createNewFile();

// Write package json file
final String customOverlayVersion = "3.3.0";
// @formatter:off
final String packageJsonContent =
"{"
+ "\"vaadin\": {"
+ "\"dependencies\": {"
+ "\"@vaadin/vaadin-dialog\": \"2.3.0\","
+ "\"@vaadin/vaadin-overlay\": \"" + customOverlayVersion + "\""
+ "}"
+ "},"
+ "\"dependencies\": {"
+ "\"@vaadin/vaadin-dialog\": \"2.3.0\","
+ "\"@vaadin/vaadin-overlay\": \"" + customOverlayVersion + "\""
+ "}"
+ "}";
// @formatter:on

FileUtils.write(packageJson, packageJsonContent,
StandardCharsets.UTF_8);

final VersionsJsonFilter versionsJsonFilter = new VersionsJsonFilter(
Json.parse(packageJsonContent), NodeUpdater.DEPENDENCIES);
// Platform defines a pinned version
TaskRunNpmInstall task = createTask(
versionsJsonFilter.getFilteredVersions(
Json.parse("{ \"@vaadin/vaadin-overlay\":\""
+ PINNED_VERSION + "\"}"))
.toJson());
task.execute();

File overlayPackageJson = new File(getNodeUpdater().nodeModulesFolder,
"@vaadin/vaadin-overlay/package.json");

// The resulting version should be the one collected from the
// annotations in the project
JsonObject overlayPackage = Json.parse(FileUtils
.readFileToString(overlayPackageJson, StandardCharsets.UTF_8));
Assert.assertEquals(customOverlayVersion,
overlayPackage.getString("version"));
}

@Override
protected String getToolName() {
return "pnpm";
Expand Down
Expand Up @@ -162,11 +162,15 @@ private void assertFilterPlatformVersions(String depKey)
Assert.assertTrue(filteredJson.hasKey("@vaadin/vaadin-upload"));
Assert.assertTrue(filteredJson.hasKey("@polymer/iron-list"));

Assert.assertEquals("1.1.2",
filteredJson.getString("@vaadin/vaadin-progress-bar"));
Assert.assertEquals("4.2.2",
Assert.assertEquals(
"'progress-bar' should be the same in package and versions",
"1.1.2", filteredJson.getString("@vaadin/vaadin-progress-bar"));
Assert.assertEquals(
"'upload' should be the same in package and versions", "4.2.2",
filteredJson.getString("@vaadin/vaadin-upload"));
Assert.assertEquals("2.0.19",
filteredJson.getString("@polymer/iron-list"));
Assert.assertEquals("'enforced' version should come from platform",
"1.5.0", filteredJson.getString("enforced"));
Assert.assertEquals("'iron-list' should be framework defined version",
"3.0.2", filteredJson.getString("@polymer/iron-list"));
}
}
2 changes: 2 additions & 0 deletions flow-server/src/test/resources/versions/package.json
Expand Up @@ -5,6 +5,7 @@
"dependencies": {
"@vaadin/vaadin-upload": "4.2.2",
"@vaadin/vaadin-progress-bar": "1.1.2",
"enforced": "1.2.0",
"@polymer/iron-list": "3.0.2"
},
"devDependencies": {
Expand All @@ -17,6 +18,7 @@
"dependencies": {
"@polymer/iron-list": "3.0.2",
"@vaadin/vaadin-progress-bar": "1.1.2",
"enforced": "1.2.0",
"@vaadin/vaadin-upload": "4.2.2"
},
"devDependencies": {
Expand Down
1 change: 1 addition & 0 deletions flow-server/src/test/resources/versions/versions.json
Expand Up @@ -2,5 +2,6 @@
"@vaadin/vaadin-progress-bar": "1.1.2",
"@vaadin/vaadin-upload": "4.2.2",
"@polymer/iron-list": "2.0.19",
"enforced": "1.5.0",
"platform": "foo"
}

0 comments on commit 9ae0550

Please sign in to comment.