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

Added support to customizable the projectFileExtensions. #19550

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,13 @@ public abstract class VaadinFlowPluginExtension @Inject constructor(private val

public abstract val cleanFrontendFiles: Property<Boolean>

/**
* The list of extra file extensions that are considered project files.
* Hashes are calculated for these files as part of detecting if a new prod
* bundle should be generated.
*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should add a note on expected extension format and a simple example list.

public abstract val extraProjectFileExtensions: ListProperty<String>
Copy link
Collaborator

@mcollovati mcollovati Jul 1, 2024

Choose a reason for hiding this comment

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

I'm still not convinced by the setting name. It still seems too generic to me.
I mean, the purpose is to add extension for frontend files that should be considered, not overall project files.
I propose something like frontendExtraFileExtensions or frontendAdditionalFileExtensions

@mshabarov what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

frontendExtraFileExtensions sounds like a good name, or frontendBuildExtraFileExtensions may be even more descriptive for developer.


public fun filterClasspath(@DelegatesTo(value = ClasspathFilter::class, strategy = Closure.DELEGATE_FIRST) block: Closure<*>) {
block.delegate = classpathFilter
block.resolveStrategy = Closure.DELEGATE_FIRST
Expand Down Expand Up @@ -427,6 +434,10 @@ public class PluginEffectiveConfiguration(

public val cleanFrontendFiles: Property<Boolean> = extension.cleanFrontendFiles
.convention(true)

public val extraProjectFileExtensions: ListProperty<String> = extension.extraProjectFileExtensions
.convention(emptyList())

/**
* Finds the value of a boolean property. It searches in gradle and system properties.
*
Expand Down Expand Up @@ -476,6 +487,7 @@ public class PluginEffectiveConfiguration(
"frontendHotdeploy=${frontendHotdeploy.get()}," +
"reactEnable=${reactEnable.get()}," +
"cleanFrontendFiles=${cleanFrontendFiles.get()}" +
"extraProjectFileExtensions=${extraProjectFileExtensions.get()}" +
")"
public companion object {
public fun get(project: Project): PluginEffectiveConfiguration =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.net.URISyntaxException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collections;
import java.util.List;
import java.util.Set;
import java.util.function.Function;
Expand Down Expand Up @@ -232,6 +233,9 @@ public abstract class FlowModeAbstractMojo extends AbstractMojo
@Parameter(property = InitParameters.REACT_ENABLE, defaultValue = "${null}")
private Boolean reactEnable;

@Parameter(defaultValue = "${null}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would add a property attribute named for example frontend.extraFileExtensions so that it could be specified also on the command line

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, the field needs Javadoc. IIRC they are used for maven goal documentation

private List<String> extraProjectFileExtensions;

/**
* Generates a List of ClasspathElements (Run and CompileTime) from a
* MavenProject.
Expand Down Expand Up @@ -522,4 +526,14 @@ public boolean isReactEnabled() {
File frontendDirectory = BuildFrontendUtil.getFrontendDirectory(this);
return FrontendUtils.isReactRouterRequired(frontendDirectory);
}

@Override
public List<String> extraProjectFileExtensions() {
if (extraProjectFileExtensions != null) {
return extraProjectFileExtensions;
}

return Collections.emptyList();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,9 @@ public static void prepareFrontend(PluginAdapterBase adapter)
.setNodeAutoUpdate(adapter.nodeAutoUpdate())
.withHomeNodeExecRequired(adapter.requireHomeNodeExec())
.setJavaResourceFolder(adapter.javaResourceFolder())
.withProductionMode(false).withReact(adapter.isReactEnabled());
.withProductionMode(false).withReact(adapter.isReactEnabled())
.withExtraProjectFileExtensions(
adapter.extraProjectFileExtensions());

// Copy jar artifact contents in TaskCopyFrontendFiles
options.copyResources(adapter.getJarFiles());
Expand Down Expand Up @@ -403,7 +405,9 @@ public static void runDevBuildNodeUpdater(PluginAdapterBuild adapter)
.withBundleBuild(true)
.skipDevBundleBuild(adapter.skipDevBundleBuild())
.withCompressBundle(adapter.compressBundle())
.withReact(adapter.isReactEnabled());
.withReact(adapter.isReactEnabled())
.withExtraProjectFileExtensions(
adapter.extraProjectFileExtensions());
new NodeTasks(options).execute();
} catch (ExecutionFailedException exception) {
throw exception;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -326,4 +326,11 @@ default Lookup createLookup(ClassFinder classFinder) {
* router and excluding React dependencies
*/
boolean isReactEnabled();

/**
* Get the list of project file extensions.
*
* @return list of project file extensions
Comment on lines +331 to +333
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should add a note on the expected extension format here (e.g. no initial dot) with a simple example list.

*/
List<String> extraProjectFileExtensions();
}
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ public class Options implements Serializable {

private boolean compressBundle = true;

private List<String> extraProjectFileExtensions = null;

/**
* The node.js version to be used when node.js is installed automatically by
* Vaadin, for example <code>"v16.0.0"</code>. Defaults to
Expand Down Expand Up @@ -954,4 +956,26 @@ public Options withCleanOldGeneratedFiles(boolean clean) {
public boolean isCleanOldGeneratedFiles() {
return cleanOldGeneratedFiles;
}

/**
* Sets the extra project file extensions.
*
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to provide additional detail on the expected extension format and perhaps a simple example

* @param extraProjectFileExtensions
* the project file extensions
* @return this builder
*/
public Options withExtraProjectFileExtensions(
List<String> extraProjectFileExtensions) {
this.extraProjectFileExtensions = extraProjectFileExtensions;
return this;
}

/**
* Gets the project file extensions.
*
* @return the project file extensions
*/
public List<String> getExtraProjectFileExtensions() {
return extraProjectFileExtensions;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,14 @@
import java.io.UncheckedIOException;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;

import org.apache.commons.io.FileUtils;
import org.apache.commons.io.IOUtils;
import org.atmosphere.util.StringEscapeUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -127,7 +131,17 @@ private void createGeneratedConfig() throws IOException {
.replace("#webComponentTags#",
webComponentTags == null || webComponentTags.isEmpty()
? ""
: String.join(";", webComponentTags));
: String.join(";", webComponentTags))
.replace("#extraProjectFileExtensions#", Optional
.ofNullable(options.getExtraProjectFileExtensions())
.orElse(Collections.emptyList()).stream().map(ext -> {
try {
return "'" + StringEscapeUtils.escapeJava(ext)
+ "'";
Comment on lines +139 to +140
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like to me that StringEscapeUtils.escapeJava does not escape single quotes, that is the only replacement that we need (e.g. ext.replace("'", "\\'")).

} catch (Exception e) {
throw new RuntimeException(e);
}
}).collect(Collectors.joining(", ")));
Copy link
Collaborator

Choose a reason for hiding this comment

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

The result of this string seems not correct; it will be appended to the existing list without a coma.

const projectFileExtensions = ['.js', '.js.map', '.ts', '.ts.map', '.tsx', '.tsx.map', '.css', '.css.map''.svg', '.ico'];

An option could be the following, for example

        String extraExtension = "";
        if (options.getExtraProjectFileExtensions() != null
                && !options.getExtraProjectFileExtensions().isEmpty()) {
            extraExtension = 
                    Optional.ofNullable(options.getExtraProjectFileExtensions())
                            .orElse(Collections.emptyList()).stream()
                            .map(ext -> ext.replace("'", "\\'"))
                            .collect(Collectors.joining("', '", ", '", "'"));
        }
        template = template.replace("#extraProjectFileExtensions#", extraExtension);

template = updateFileSystemRouterVitePlugin(template);

FileIOUtils.writeIfChanged(generatedConfigFile, template);
Expand Down
2 changes: 1 addition & 1 deletion flow-server/src/main/resources/vite.generated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ function statsExtracterPlugin(): PluginOption {

const frontendFiles: Record<string, string> = {};

const projectFileExtensions = ['.js', '.js.map', '.ts', '.ts.map', '.tsx', '.tsx.map', '.css', '.css.map'];
const projectFileExtensions = ['.js', '.js.map', '.ts', '.ts.map', '.tsx', '.tsx.map', '.css', '.css.map'#extraProjectFileExtensions#];

const isThemeComponentsResource = (id: string) =>
id.startsWith(themeOptions.frontendGeneratedFolder.replace(/\\/g, '/'))
Expand Down
Loading