-
Notifications
You must be signed in to change notification settings - Fork 883
Fix broken npx goal when using a proxy #1120
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
base: master
Are you sure you want to change the base?
Conversation
|
This PR should also fix #1105 since yarn accepts the same syntax. |
414d2d8 to
7b2e67c
Compare
|
@eirslett can you take a look when you have time? 😃 |
|
I agree that the extra npm arguments are being added on the wrong side of the command that npx is running. The Javadoc for the NpxRunner states As this issue states, the reverse is happening. Here's a reproducible case to demonstrate <?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<groupId>org.example</groupId>
<artifactId>frontend-maven-plugin-example</artifactId>
<version>1.0.0-SNAPSHOT</version>
<packaging>pom</packaging>
<build>
<plugins>
<plugin>
<groupId>com.github.eirslett</groupId>
<artifactId>frontend-maven-plugin</artifactId>
<version>1.15.0</version>
<configuration>
<environmentVariables>
<npm_config_cache>${project.build.directory}/_npx</npm_config_cache>
</environmentVariables>
<installDirectory>${project.build.directory}</installDirectory>
<nodeVersion>v20.12.2</nodeVersion>
</configuration>
<executions>
<execution>
<id>install-node</id>
<goals>
<goal>install-node-and-npm</goal>
</goals>
<phase>initialize</phase>
</execution>
<execution>
<id>run-npx</id>
<goals>
<goal>npx</goal>
</goals>
<phase>process-resources</phase>
<configuration>
<arguments>--yes --package js-yaml js-yaml test.yml</arguments>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</project>First, create the test.yml file to load. test.yml If you run the following command, it will work: However, if you pass an npm registry URL: here's what it will run: Notice that the There are many commands that don't recognize these extra arguments that are supposed to be passed to npm itself. Here's how the command should look to solve this problem: There's also no need for the |
|
Any chance this will get merged/fixed anytime soon? Over at the Apache Guacamole project we're using frontend-maven-plugin to run npm/npx commands, and hitting this issue with some users who have proxies configured in their environment. |
Summary
This PR
fixes #1030
fixes #1121
fixes #1105
in the current version of the plugin the npx goal is broken when using a proxy.
As you can read in the npm documentation npx does not support the syntax that was used by the plugin before:
https://docs.npmjs.com/cli/v8/commands/npm-exec#npx-vs-npm-exec
syntax before:
npx @sompackage arg1 arg2 -- --proxy=someproxy --https-proxy=someproxy --registry=someregistrysyntax after:
npx --proxy=someproxy --https-proxy=someproxy --registry=someregistry @somepackage arg1 arg2npmsupports both syntax so no harm done in switching it.Tests and Documentation
I've changed the 3 existing tests to check for the new syntax.
Please let me know if you need other changes. I'll try to get to it asap 😃