Skip to content

core(plugins): support scoped npm packages #16550

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

Merged
merged 1 commit into from
Jun 25, 2025

Conversation

demostanis
Copy link
Contributor

Summary

currently we can't use plugins that are namespaced, e.g. @myorg/lighthouse-plugin-blabla

im simply splitting on / and checking if the second part starts with the correct prefix.

Related Issues/PRs

@demostanis demostanis requested a review from a team as a code owner June 23, 2025 14:44
@demostanis demostanis requested review from paulirish and removed request for a team June 23, 2025 14:44
Copy link

google-cla bot commented Jun 23, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@demostanis demostanis changed the title fix: don't error out on npm-namespaced plugin names core: don't error out on npm-namespaced plugin names Jun 23, 2025
@connorjclark
Copy link
Collaborator

Thanks!

Will need a couple things first before landing this:

  1. please run yarn lint --fix
  2. need a test in core/test/config/validation-test.js – .assertValidPluginName

@demostanis
Copy link
Contributor Author

done

@connorjclark connorjclark changed the title core: don't error out on npm-namespaced plugin names core(plugins): support scoped npm packages Jun 24, 2025
@connorjclark
Copy link
Collaborator

Linting is failing. I tried pushing to the forked branch but for some reason GitHub is not allowing me. Here is what I wanted to patch in.

commit ad02d098c368bd760bdf9fb8e6bbd89063c0cd2c
Author: Connor Clark <cjamcl@gmail.com>
Date:   Tue Jun 24 11:01:14 2025 -0700

    lint

diff --git a/core/config/validation.js b/core/config/validation.js
index 7f33cc157..eb1ae0795 100644
--- a/core/config/validation.js
+++ b/core/config/validation.js
@@ -36,9 +36,10 @@ function isValidArtifactDependency(dependent, dependency) {
  * @param {string} pluginName
  */
 function assertValidPluginName(config, pluginName) {
-  const parts = pluginName.split("/");
-  if (parts.length === 2)
+  const parts = pluginName.split('/');
+  if (parts.length === 2) {
     pluginName = parts[1];
+  }
   if (!pluginName.startsWith('lighthouse-plugin-')) {
     throw new Error(`plugin name '${pluginName}' does not start with 'lighthouse-plugin-'`);
   }
diff --git a/core/test/config/validation-test.js b/core/test/config/validation-test.js
index 1cdb07bf1..30e28fa23 100644
--- a/core/test/config/validation-test.js
+++ b/core/test/config/validation-test.js
@@ -62,12 +62,14 @@ describe('Config Validation', () => {
     });
 
     it('should not throw if plugin starts with lighthouse-plugin', () => {
-      const invocation = () => validation.assertValidPluginName(defaultConfig, 'lighthouse-plugin-test');
+      const invocation =
+        () => validation.assertValidPluginName(defaultConfig, 'lighthouse-plugin-test');
       expect(invocation).not.toThrow();
     });
 
     it('should not throw if plugin starts with @xxx/lighthouse-plugin', () => {
-      const invocation = () => validation.assertValidPluginName(defaultConfig, '@org/lighthouse-plugin-test');
+      const invocation =
+        () => validation.assertValidPluginName(defaultConfig, '@org/lighthouse-plugin-test');
       expect(invocation).not.toThrow();
     });
 

@demostanis
Copy link
Contributor Author

okay, should be good now, i applied your patch
i shouldve checked lint errors twice, sorry for wasting your time!

@connorjclark
Copy link
Collaborator

Thanks!

@connorjclark connorjclark merged commit 78f72e9 into GoogleChrome:main Jun 25, 2025
21 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants