Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Maven plugin improvements #20695

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
57a0ead
Change all occurences of private to protected
AB-xdev Dec 13, 2024
bac89bf
Only use ``BuildContext`` where it's needed
AB-xdev Dec 13, 2024
dce600d
Assume that ``BuildContext`` can be null
AB-xdev Dec 13, 2024
4d5c23c
Remove ``productionMode`` check
AB-xdev Dec 13, 2024
a127fa1
Log accurately how long a goal needs for execution
AB-xdev Dec 13, 2024
c213b7a
Introduce new Reflector system
AB-xdev Dec 13, 2024
dcfc957
Remove unused code
AB-xdev Dec 13, 2024
6cf2c8a
Introduce optimization for FlowModeAbstractMojo
AB-xdev Dec 13, 2024
39ff81f
Cleanup
AB-xdev Dec 13, 2024
f8cc082
Introduce optimization for BuildFrontendMojo
AB-xdev Dec 13, 2024
6d01e74
Cleanup
AB-xdev Dec 13, 2024
f30e3cd
Remove not needed dependency
AB-xdev Dec 13, 2024
61a6e14
Fix tests
AB-xdev Dec 13, 2024
98c7342
Fix test - Use same logik as in plugin
AB-xdev Dec 13, 2024
7d9808f
Also set default values to fields (not just annotations) so that test…
AB-xdev Dec 13, 2024
01a7983
Fix format
AB-xdev Dec 13, 2024
9193f12
Fix format
AB-xdev Dec 13, 2024
782b288
Rename ``ReflectorController`` -> ``ReflectorProvider``
AB-xdev Jan 14, 2025
dc28229
Cleanup
AB-xdev Jan 14, 2025
9256be9
Rework config
AB-xdev Jan 14, 2025
288be36
Use method instead of field access
AB-xdev Jan 14, 2025
37e7afd
Rename ``of`` -> ``createNew``
AB-xdev Jan 14, 2025
137d820
Use simpler if instead of Optional
AB-xdev Jan 14, 2025
3f5ed55
Update docs
AB-xdev Jan 14, 2025
1fc551a
Restore unused code and deprecate
AB-xdev Jan 14, 2025
390e08f
Make constants configurable
AB-xdev Jan 14, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions flow-plugins/flow-maven-plugin/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,6 @@
<version>3.4.2</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>com.google.gwt</groupId>
<artifactId>gwt-elemental</artifactId>
<exclusions>
<exclusion>
<groupId>com.google.gwt</groupId>
<artifactId>gwt-user</artifactId>
</exclusion>
</exclusions>
</dependency>

<dependency>
<groupId>org.apache.maven.plugin-tools</groupId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.net.URISyntaxException;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.TimeoutException;
import java.util.function.Consumer;

Expand Down Expand Up @@ -70,35 +71,35 @@ public class BuildFrontendMojo extends FlowModeAbstractMojo
* Whether to generate a bundle from the project frontend sources or not.
*/
@Parameter(defaultValue = "true")
private boolean generateBundle;
protected boolean generateBundle;

/**
* Whether to run <code>npm install</code> after updating dependencies.
*/
@Parameter(defaultValue = "true")
private boolean runNpmInstall;
protected boolean runNpmInstall;

/**
* Whether to generate embeddable web components from WebComponentExporter
* inheritors.
*/
@Parameter(defaultValue = "true")
private boolean generateEmbeddableWebComponents;
protected boolean generateEmbeddableWebComponents;

/**
* Defines the project frontend directory from where resources should be
* copied from for use with the frontend build tool.
*/
@Parameter(defaultValue = "${project.basedir}/"
+ Constants.LOCAL_FRONTEND_RESOURCES_PATH)
private File frontendResourcesDirectory;
protected File frontendResourcesDirectory;

/**
* Whether to use byte code scanner strategy to discover frontend
* components.
*/
@Parameter(defaultValue = "true")
private boolean optimizeBundle;
protected boolean optimizeBundle;

/**
* Setting this to true will run {@code npm ci} instead of
Expand All @@ -111,7 +112,7 @@ public class BuildFrontendMojo extends FlowModeAbstractMojo
* overwritten and production builds are reproducible.
*/
@Parameter(property = InitParameters.CI_BUILD, defaultValue = "false")
private boolean ciBuild;
protected boolean ciBuild;

/**
* Setting this to {@code true} will force a build of the production build
Expand All @@ -121,7 +122,7 @@ public class BuildFrontendMojo extends FlowModeAbstractMojo
* {@link #optimizeBundle} parameter.
*/
@Parameter(property = InitParameters.FORCE_PRODUCTION_BUILD, defaultValue = "false")
private boolean forceProductionBuild;
protected boolean forceProductionBuild;

/**
* Control cleaning of generated frontend files when executing
Expand All @@ -130,13 +131,29 @@ public class BuildFrontendMojo extends FlowModeAbstractMojo
* Mainly this is wanted to be true which it is by default.
*/
@Parameter(property = InitParameters.CLEAN_BUILD_FRONTEND_FILES, defaultValue = "true")
private boolean cleanFrontendFiles;
protected boolean cleanFrontendFiles;

/**
* <b>Only disable this if you have nothing from Vaadin to license!</b>
* <p>
* Otherwise there might be unexpected build problems and you will get into legal trouble.
* </p>
*/
@Parameter(defaultValue = "true")
protected boolean performLicenseCheck = true;

/**
* Determines if the runtime dependencies should be checked.
* <p>
* Usually the check is only required after a Vaadin version update.
* </p>
*/
@Parameter(defaultValue = "true")
protected boolean checkRuntimeDependency = true;

@Override
protected void executeInternal()
throws MojoExecutionException, MojoFailureException {
long start = System.nanoTime();

TaskCleanFrontendFiles cleanTask = new TaskCleanFrontendFiles(
npmFolder(), frontendDirectory(), getClassFinder());
try {
Expand All @@ -154,18 +171,18 @@ protected void executeInternal()
cleanTask.execute();
}
} catch (URISyntaxException | TimeoutException
| ExecutionFailedException exception) {
| ExecutionFailedException exception) {
throw new MojoExecutionException(exception.getMessage(),
exception);
}
}
LicenseChecker.setStrictOffline(true);
boolean licenseRequired = BuildFrontendUtil.validateLicenses(this);

BuildFrontendUtil.updateBuildFile(this, licenseRequired);
if (performLicenseCheck) {
LicenseChecker.setStrictOffline(true);
}
boolean licenseRequired = performLicenseCheck && BuildFrontendUtil.validateLicenses(this);

long ms = (System.nanoTime() - start) / 1000000;
getLog().info("Build frontend completed in " + ms + " ms.");
BuildFrontendUtil.updateBuildFile(this, licenseRequired);
}

/**
Expand Down Expand Up @@ -195,31 +212,26 @@ protected boolean cleanFrontendFiles() {

@Override
public File frontendResourcesDirectory() {

return frontendResourcesDirectory;
}

@Override
public boolean generateBundle() {

return generateBundle;
}

@Override
public boolean generateEmbeddableWebComponents() {

return generateEmbeddableWebComponents;
}

@Override
public boolean optimizeBundle() {

return optimizeBundle;
}

@Override
public boolean runNpmInstall() {

return runNpmInstall;
}

Expand All @@ -240,46 +252,49 @@ public boolean compressBundle() {

@Override
public boolean checkRuntimeDependency(String groupId, String artifactId,
Consumer<String> missingDependencyMessage) {
Consumer<String> missingDependencyMessage) {
if (!checkRuntimeDependency) {
getLog().info("Ignoring runtime dependency check");
return true;
}

Objects.requireNonNull(groupId, "groupId cannot be null");
Objects.requireNonNull(artifactId, "artifactId cannot be null");
if (missingDependencyMessage == null) {
missingDependencyMessage = text -> {
};
}
Comment on lines -246 to -249
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would instantiating an Optional be more performant than having a no-op consumer?
Otherwise, I would leave the code as is.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used an Optional further down in the ifs as I found it more readable and it makes more sense there (the previously created consumer may have never been used in the ifs below).

As an alternative one can also do a null check, like:

if(...) {
  if (missingDependencyMessage != null) { // Potential replacement for Optional
    missingDependencyMessage.accept(...);
  }
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, I would prefer the null check in this case, since there is no pipeline built on top of the Optional.


List<Artifact> deps = project.getArtifacts().stream()
final List<Artifact> deps = project.getArtifacts().stream()
.filter(artifact -> groupId.equals(artifact.getGroupId())
&& artifactId.equals(artifact.getArtifactId()))
.toList();
if (deps.isEmpty()) {
missingDependencyMessage.accept(String.format(
"""
The dependency %1$s:%2$s has not been found in the project configuration.
Please add the following dependency to your POM file:

<dependency>
<groupId>%1$s</groupId>
<artifactId>%2$s</artifactId>
<scope>runtime</scope>
</dependency>
""",
groupId, artifactId));
Optional.ofNullable(missingDependencyMessage)
.ifPresent(c -> c.accept(String.format(
"""
The dependency %1$s:%2$s has not been found in the project configuration.
Please add the following dependency to your POM file:

<dependency>
<groupId>%1$s</groupId>
<artifactId>%2$s</artifactId>
<scope>runtime</scope>
</dependency>
""",
groupId, artifactId)));
return false;
} else if (deps.stream().noneMatch(artifact -> !artifact.isOptional()
&& artifact.getArtifactHandler().isAddedToClasspath()
&& (Artifact.SCOPE_COMPILE.equals(artifact.getScope())
|| Artifact.SCOPE_PROVIDED.equals(artifact.getScope())
|| Artifact.SCOPE_RUNTIME
.equals(artifact.getScope())))) {
missingDependencyMessage.accept(String.format(
"""
The dependency %1$s:%2$s has been found in the project configuration,
but with a scope that does not guarantee its presence at runtime.
Please check that the dependency has 'compile', 'provided' or 'runtime' scope.
To check the current dependency scope, you can run 'mvn dependency:tree -Dincludes=%1$s:%2$s'
""",
groupId, artifactId));
|| Artifact.SCOPE_PROVIDED.equals(artifact.getScope())
|| Artifact.SCOPE_RUNTIME
.equals(artifact.getScope())))) {
Optional.ofNullable(missingDependencyMessage)
.ifPresent(c -> c.accept(String.format(
"""
The dependency %1$s:%2$s has been found in the project configuration,
but with a scope that does not guarantee its presence at runtime.
Please check that the dependency has 'compile', 'provided' or 'runtime' scope.
To check the current dependency scope, you can run 'mvn dependency:tree -Dincludes=%1$s:%2$s'
""",
groupId, artifactId)));
return false;
}
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,20 @@ public class ConvertPolymerMojo extends FlowModeAbstractMojo {
* folder.
*/
@Parameter(property = "vaadin.path")
private String path;
protected String path;

/**
* Whether to enforce Lit 1 compatible imports.
*/
@Parameter(property = "vaadin.useLit1", defaultValue = "${false}")
private boolean useLit1;
protected boolean useLit1;

/**
* Whether to disable the usage of the JavaScript optional chaining operator
* (?.) in the output.
*/
@Parameter(property = "vaadin.disableOptionalChaining", defaultValue = "${false}")
private boolean disableOptionalChaining;
protected boolean disableOptionalChaining;

@Override
protected void executeInternal() throws MojoFailureException {
Expand Down
Loading
Loading