Fix the order of additional arguments for npx.

fixes eirslett#1030
123Haynes · Nov 4, 2023 · 7b2e67c · 7b2e67c
1 parent 4045aaf
commit 7b2e67c
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 9 deletions.
diff --git a/...in-core/src/main/java/com/github/eirslett/maven/plugins/frontend/lib/ArgumentsParser.java b/...in-core/src/main/java/com/github/eirslett/maven/plugins/frontend/lib/ArgumentsParser.java
@@ -39,6 +39,7 @@ List<String> parse(String args) {
         }
 
         final List<String> arguments = new LinkedList<>();
+        final List<String> allArguments = new LinkedList<>();
         final StringBuilder argumentBuilder = new StringBuilder();
         Character quote = null;
 
@@ -64,13 +65,15 @@ List<String> parse(String args) {
 
         addArgument(argumentBuilder, arguments);
 
+        // Prepend additionalArguments before the other arguments
         for (String argument : this.additionalArguments) {
             if (!arguments.contains(argument)) {
-                arguments.add(argument);
+                allArguments.add(argument);
             }
         }
+        allArguments.addAll(arguments);
 
-        return new ArrayList<>(arguments);
+        return new ArrayList<>(allArguments);
     }
 
     private static void addArgument(StringBuilder argumentBuilder, List<String> arguments) {

diff --git a/...d-plugin-core/src/main/java/com/github/eirslett/maven/plugins/frontend/lib/NpxRunner.java b/...d-plugin-core/src/main/java/com/github/eirslett/maven/plugins/frontend/lib/NpxRunner.java
@@ -16,10 +16,10 @@ public DefaultNpxRunner(NodeExecutorConfig config, ProxyConfig proxyConfig, Stri
 
     // Visible for testing only.
     /**
-     * These are, in fact, npm arguments, that need to be split from the npx arguments by '--'.
+     * These are, in fact, npm arguments, that need to come before the package arguments.
      *
      * See an example:
-     * npx some-package -- --registry=http://myspecialregisty.com
+     * npx --registry=http://myspecialregisty.com some-package
      */
     static List<String> buildNpmArguments(ProxyConfig proxyConfig, String npmRegistryURL) {
         List<String> arguments = new ArrayList<>();
@@ -51,7 +51,6 @@ static List<String> buildNpmArguments(ProxyConfig proxyConfig, String npmRegistr
             npmArguments = arguments;
         } else {
             npmArguments = new ArrayList<>();
-            npmArguments.add("--");
             npmArguments.addAll(arguments);
         }
 

diff --git a/...ore/src/test/java/com/github/eirslett/maven/plugins/frontend/lib/ArgumentsParserTest.java b/...ore/src/test/java/com/github/eirslett/maven/plugins/frontend/lib/ArgumentsParserTest.java
@@ -60,13 +60,13 @@ public void repeatedArgumentsAreAccepted() {
     public void testAdditionalArgumentsNoIntersection() {
         ArgumentsParser parser = new ArgumentsParser(Arrays.asList("foo", "bar"));
 
-        assertArrayEquals(new Object[] { "foobar", "foo", "bar" }, parser.parse("foobar").toArray());
+        assertArrayEquals(new Object[] { "foo", "bar", "foobar" }, parser.parse("foobar").toArray());
     }
 
     @Test
     public void testAdditionalArgumentsWithIntersection() {
         ArgumentsParser parser = new ArgumentsParser(Arrays.asList("foo", "foobar"));
 
-        assertArrayEquals(new Object[] { "bar", "foobar", "foo" }, parser.parse("bar foobar").toArray());
+        assertArrayEquals(new Object[] { "foo", "bar", "foobar" }, parser.parse("bar foobar").toArray());
     }
 }
diff --git a/...re/src/test/java/com/github/eirslett/maven/plugins/frontend/lib/DefaultNpxRunnerTest.java b/...re/src/test/java/com/github/eirslett/maven/plugins/frontend/lib/DefaultNpxRunnerTest.java
@@ -23,7 +23,7 @@ public void buildArgument_basicTest() {
     @Test
     public void buildArgument_withRegistryUrl() {
         List<String> arguments = DefaultNpxRunner.buildNpmArguments(new ProxyConfig(Collections.emptyList()), registryUrl);
-        Assertions.assertEquals(2, arguments.size());
-        assertThat(arguments, CoreMatchers.hasItems("--", "--registry=" + registryUrl));
+        Assertions.assertEquals(1, arguments.size());
+        assertThat(arguments, CoreMatchers.hasItems("--registry=" + registryUrl));
     }
 }