eirslett / frontend-maven-plugin

"Maven-node-grunt-gulp-npm-node-plugin to end all maven-node-grunt-gulp-npm-plugins." A Maven plugin that downloads/installs Node and NPM locally, runs NPM install, Grunt, Gulp and/or Karma.
Apache License 2.0
4.21k stars 867 forks source link

ensure mojos are ignoring pom packaging #1009

Closed rmannibucau closed 2 years ago

rmannibucau commented 2 years ago

Summary

For a hierachy of modules using the same lifecycle (install node, npm install etc), it is today hard to define it until you do a maven extension whereas defining it in the parent would be convenient. This PR defines ignored packaging - default to pom - which enables this use case.

Tests and Documentation

Didn't write yet cause I think an IT is too hard to write and didn't find any unit test so any help if you think it is worth it for such a simple test would be welcomed.

Sample usage

<plugin>
  <groupId>com.github.eirslett</groupId>
  <artifactId>frontend-maven-plugin</artifactId>
  <version>1.12.1-SNAPSHOT</version>
  <executions>
    <execution>
      <id>install-node-npm</id>
      <phase>generate-resources</phase>
      <goals>
        <goal>install-node-and-npm</goal>
      </goals>
      <configuration> <!-- install it once in the parent -->
        <installDirectory>${project.basedir}/.node</installDirectory>
        <nodeVersion>v17.0.1</nodeVersion>
        <npmVersion>8.1.0</npmVersion>
        <ignoredPackagings> <!-- ignore in children -->
          <ignoredPackaging>jar</ignoredPackaging>
        </ignoredPackagings>
      </configuration>
    </execution>
    <execution>
      <id>npm-install</id>
      <phase>process-classes</phase>
      <goals>
        <goal>npm</goal>
      </goals>
    </execution>
    <execution>
      <id>npm-build</id>
      <phase>process-classes</phase>
      <goals>
        <goal>npm</goal>
      </goals>
      <configuration>
        <arguments>run build</arguments>
      </configuration>
    </execution>
  </executions>
  <configuration> <!-- for children, reuse parent installation of node and run the build in child module -->
    <installDirectory>${project.parent.basedir}/.node</installDirectory>
    <workingDirectory>${project.basedir}</workingDirectory>
  </configuration>
</plugin>
rmannibucau commented 2 years ago

Any way to get this PR reviewed/approved/merged and released? Would be very helpful, thanks.

eirslett commented 2 years ago

In general I'm quite hesitant to add features like this. The reason is: I think such concerns (placement of configuration, reuse, DRY etc.) are the responsibility of the build tool (Maven). While the plugins for Maven should be concerned with doing what each plugin is specialized for - in this case, managing and running Node and npm.

If you want a more "convenient" build tool, I would suggest Gradle, which is more flexible and allows for much of the flexibility you are looking for. It has gradle-node-plugin which is quite similar to frontend-maven-plugin. Maven is more convenient in the sense that it's more widely used (Maven is "everywhere"), but then its main problem is the huge amount of XML you will have to write.

bmarwell commented 2 years ago

@eirslett this is VERY inconvenient. Please be open for discussion and don't just say "use another tool". This is how maven works, and I find it not very pleasing that you like gradle better and therefore close this issue.

What Romain suggested is a very common things in maven plugins, e.g. the openliberty plugin will only run on .war/.ear packaging projects.

rmannibucau commented 2 years ago

I think such concerns (placement of configuration, reuse, DRY etc.) are the responsibility of the build tool (Maven).

Well, most plugins already have this feature, same as supporting skip flag where you could argue the same point but it is not the way maven is designed so overall, even if I tend to agree with the reasoning, I think it is better to align on the build tool you target than saying it is not complete and let the end users with an incomplete feature set, no?

If you want a more "convenient" build tool, I would suggest Gradle, which is more flexible and allows for much of the flexibility you are looking for. It has gradle-node-plugin which is quite similar to frontend-maven-plugin.

Well, it is a bit out of topic IMHO - and don't want to "thread" this topic there, but gradle has its drawbacks too and to have tested this path on multiple project it is clearly a no-go - maybe for my case but saying to change all the end user choices is not really a solution, just for your information, here are the reasons on my side:

  1. you move the responsability of the build to a script which is NOT portable NOR well supported by IDE (literally lost hours per day moving to gradle due to idea integration)
  2. the build is NOT faster - in particular with mvnd, this is a myth
  3. build with the daemon has false positive (green build cause it didn't trigger properly transitive modules
  4. API is not that stable breaking the build upgrading gradle quite easily even if some effort were done
  5. very few dev in enterprises know gradle compared to maven (at least there) so can only lead to issues

its main problem is the huge amount of XML you will have to write

Likely out of topic too but you can reduce it by writing some extension which are designed for that. But I'd like to emphasis the point here is that the frontend-maven-plugin does not enable to reuse maven inheritance due to the packaging ignorance (this very particular ticket/pr).

So really think it is worth the 6 lines of code and avoid forks/wrappers ;).

Hope it makes sense.

rmannibucau commented 2 years ago

FYI I drafted quickly a PoC to workaround this blocker https://github.com/rmannibucau/hierarchy-extension, will try to release a 0.0.1 soon but still, would be saner for the plugin - and end users - to align on maven common practise probably IMHO.

Edit: another 2 options to solve this issue with plain maven (if someone reads it later):

  1. use a profile with an activation on file using package.json as activaton (if possible)
  2. use pluginManagement block to define children configuration and redefine it in the parent adding <inherited>false</> flag to the execution (same id) defined in the parent + binding it on phase=none in the parent.