Skip to content

Commit 44f86ae

Browse files
Fix NPM plugin module resolution (#252)
v3.4.0 installs NPM plugins from the consumer workspace, then imports them from the Find action. The install succeeds, but Node can't resolve the package from `dist/pluginManager/pluginNpmLoader.js`. This happened in [the alt text plugin workflow](https://github.com/github/accessibility-scanner-alt-text-plugin/actions/runs/30406827436) after switching to the new package object in #62. This installs the package beside the loader module with an explicit npm prefix. In the action that's `dist/pluginManager/node_modules`, so the bare ESM import finds it without using `GITHUB_WORKSPACE`. Keeping the install out of the Find action root also avoids npm re-resolving the action's own dependencies. The allowlist, version pinning, `--ignore-scripts`, `--no-save`, `--no-package-lock`, duplicate handling, and warnings are unchanged. I tested the exact input from #62: ```yaml scans: | ["axe", {"name": "alt-text-scan", "package": "@github/accessibility-scanner-alt-text-plugin", "version": "1.1.0"}] ``` In a clean scanner copy with a separate consumer cwd, the published 1.1.0 package loaded from the compiled action. A local page produced both the plugin's `placeholder-alt-text` finding and Axe's `button-name` finding. The Find action's dependency versions were unchanged after the install. Also ran all 82 action tests, lint, format check, and the Find build. I updated the scanner docs that still showed plugin 1.0.0 or described `scans` as strings only. This needs a scanner v3.4.1 after merge. The plugin stays on v1.1.0, then its workflow pin can move from the v3.4.0 commit to the v3.4.1 commit.
2 parents 309ccd0 + 79266f4 commit 44f86ae

8 files changed

Lines changed: 84 additions & 9 deletions

File tree

‎.github/actions/find/README.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,9 @@ configuration option.
3737

3838
#### `scans`
3939

40-
**Optional** Stringified JSON array of scans (string) to perform. If not provided, only Axe will be performed.
40+
**Optional** Stringified JSON array of scans to perform. Core engines and local plugins use string names.
41+
Allowlisted NPM plugins use an object with `name`, `package`, and optional `version`. If not provided, only Axe
42+
will be performed. See [the plugin docs](../../../PLUGINS.md#loading-plugins-from-npm-packages) for an example.
4143

4244
### Outputs
4345

‎.github/actions/find/src/pluginManager/pluginNpmLoader.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,16 @@
11
import {execFileSync} from 'child_process'
2+
import {fileURLToPath} from 'url'
23
import * as core from '@actions/core'
34
import type {NpmPluginRequest, Plugin} from './types.js'
45

6+
const pluginRoot = fileURLToPath(new URL('.', import.meta.url))
7+
58
// Install the package at runtime.
69
export function installNpmPackage(spec: string) {
7-
execFileSync('npm', ['install', spec, '--no-save', '--no-package-lock', '--ignore-scripts'], {stdio: 'inherit'})
10+
execFileSync('npm', ['install', spec, '--prefix', pluginRoot, '--no-save', '--no-package-lock', '--ignore-scripts'], {
11+
cwd: pluginRoot,
12+
stdio: 'inherit',
13+
})
814
}
915

1016
// Install and import a single NPM-published plugin

‎.github/actions/find/tests/findForUrl.test.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,11 +163,19 @@ describe('findForUrl', () => {
163163

164164
it('runs plugins when a scans entry is an object-form NPM plugin', async () => {
165165
loadedPlugins = []
166-
actionInput = JSON.stringify([{name: 'alt-text-scan', package: '@github/accessibility-scanner-alt-text-plugin'}])
166+
actionInput = JSON.stringify([
167+
'axe',
168+
{
169+
name: 'alt-text-scan',
170+
package: '@github/accessibility-scanner-alt-text-plugin',
171+
version: '1.1.0',
172+
},
173+
])
167174
clearAll()
168175

169176
await findForUrl('test.com')
170177
expect(pluginManager.loadPlugins).toHaveBeenCalledTimes(1)
178+
expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(1)
171179
})
172180
})
173181

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import * as fs from 'fs'
2+
import * as os from 'os'
3+
import * as path from 'path'
4+
import {fileURLToPath} from 'url'
5+
import {describe, expect, it} from 'vitest'
6+
7+
import {loadPluginViaNpm} from '../src/pluginManager/pluginNpmLoader.js'
8+
9+
const PLUGIN_ROOT = fileURLToPath(new URL('../src/pluginManager/', import.meta.url))
10+
const PLUGIN_NODE_MODULES = path.join(PLUGIN_ROOT, 'node_modules')
11+
12+
describe('npmPluginLoader integration', () => {
13+
it('installs and loads a released plugin outside the consumer workspace', {timeout: 120_000}, async () => {
14+
const originalCwd = process.cwd()
15+
const originalMinimumReleaseAge = process.env.npm_config_min_release_age
16+
const consumerWorkspace = fs.mkdtempSync(path.join(os.tmpdir(), 'accessibility-scanner-consumer-'))
17+
18+
try {
19+
process.chdir(consumerWorkspace)
20+
process.env.npm_config_min_release_age = '0'
21+
expect(process.cwd()).not.toBe(PLUGIN_ROOT)
22+
23+
const plugin = await loadPluginViaNpm({
24+
name: 'alt-text-scan',
25+
package: '@github/accessibility-scanner-alt-text-plugin',
26+
version: '1.1.0',
27+
})
28+
29+
expect(plugin?.name).toBe('alt-text-scan')
30+
expect(plugin?.default).toBeTypeOf('function')
31+
expect(
32+
fs.existsSync(
33+
path.join(PLUGIN_NODE_MODULES, '@github', 'accessibility-scanner-alt-text-plugin', 'package.json'),
34+
),
35+
).toBe(true)
36+
} finally {
37+
process.chdir(originalCwd)
38+
if (originalMinimumReleaseAge === undefined) {
39+
delete process.env.npm_config_min_release_age
40+
} else {
41+
process.env.npm_config_min_release_age = originalMinimumReleaseAge
42+
}
43+
fs.rmSync(consumerWorkspace, {recursive: true, force: true})
44+
fs.rmSync(PLUGIN_NODE_MODULES, {recursive: true, force: true})
45+
}
46+
})
47+
})

‎.github/actions/find/tests/pluginNpmLoader.test.ts‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {describe, it, expect, vi, beforeEach} from 'vitest'
22

33
import * as childProcess from 'child_process'
4+
import {fileURLToPath} from 'url'
45
import * as core from '@actions/core'
56
import * as pluginManager from '../src/pluginManager/index.js'
67
import * as npmPluginLoader from '../src/pluginManager/pluginNpmLoader.js'
@@ -13,6 +14,7 @@ vi.mock('../src/pluginManager/pluginNpmLoader.js', {spy: true})
1314
vi.mock('../src/scansContextProvider.js', {spy: true})
1415

1516
const ALLOWED = '@github/accessibility-scanner-alt-text-plugin'
17+
const PLUGIN_ROOT = fileURLToPath(new URL('../src/pluginManager/', import.meta.url))
1618

1719
function mockNpmPlugins(npmPlugins: NpmPluginRequest[]) {
1820
vi.spyOn(scansContextProvider, 'getScansContext').mockReturnValue({
@@ -35,8 +37,9 @@ describe('npmPluginLoader', () => {
3537
npmPluginLoader.installNpmPackage('some-pkg@1.0.0')
3638
expect(execSpy).toHaveBeenCalledWith(
3739
'npm',
38-
['install', 'some-pkg@1.0.0', '--no-save', '--no-package-lock', '--ignore-scripts'],
40+
['install', 'some-pkg@1.0.0', '--prefix', PLUGIN_ROOT, '--no-save', '--no-package-lock', '--ignore-scripts'],
3941
{
42+
cwd: PLUGIN_ROOT,
4043
stdio: 'inherit',
4144
},
4245
)
@@ -49,8 +52,16 @@ describe('npmPluginLoader', () => {
4952
await npmPluginLoader.loadPluginViaNpm({name: 'p', package: 'nonexistent-pkg-xyz', version: '2.3.4'})
5053
expect(execSpy).toHaveBeenCalledWith(
5154
'npm',
52-
['install', 'nonexistent-pkg-xyz@2.3.4', '--no-save', '--no-package-lock', '--ignore-scripts'],
53-
{stdio: 'inherit'},
55+
[
56+
'install',
57+
'nonexistent-pkg-xyz@2.3.4',
58+
'--prefix',
59+
PLUGIN_ROOT,
60+
'--no-save',
61+
'--no-package-lock',
62+
'--ignore-scripts',
63+
],
64+
{cwd: PLUGIN_ROOT, stdio: 'inherit'},
5465
)
5566
})
5667

‎PLUGINS.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ jobs:
4646
## Loading plugins from NPM packages
4747

4848
In addition to local plugins under `./.github/scanner-plugins`, the scanner can install and load plugins published as NPM packages. This avoids having to vendor a plugin's source into your repo.
49+
NPM package loading requires scanner v3.4.1 or later.
4950

5051
To use an NPM plugin, pass an object (instead of a plain string) in the `scans` input with the following fields:
5152

@@ -64,7 +65,7 @@ jobs:
6465
- uses: github/accessibility-scanner@v3
6566
with:
6667
scans: |
67-
["axe", {"name": "alt-text-scan", "package": "@github/accessibility-scanner-alt-text-plugin", "version": "1.0.0"}]
68+
["axe", {"name": "alt-text-scan", "package": "@github/accessibility-scanner-alt-text-plugin", "version": "1.1.0"}]
6869
```
6970
7071
Notes:

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ The [Alt Text Plugin](https://github.com/github/accessibility-scanner-alt-text-p
188188

189189
```yaml
190190
scans: |
191-
["axe", {"name": "alt-text-scan", "package": "@github/accessibility-scanner-alt-text-plugin", "version": "1.0.0"}]
191+
["axe", {"name": "alt-text-scan", "package": "@github/accessibility-scanner-alt-text-plugin", "version": "1.1.0"}]
192192
```
193193
194194
See the [plugin README](https://github.com/github/accessibility-scanner-alt-text-plugin#getting-started) for the current release version, full rule list, and setup instructions.

‎action.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ inputs:
6464
description: 'Playwright colorScheme setting: https://playwright.dev/docs/api/class-browser#browser-new-context-option-color-scheme'
6565
required: false
6666
scans:
67-
description: 'Stringified JSON array of scans to perform. If not provided, only Axe will be performed'
67+
description: "Stringified JSON array of scans to perform. Core engines and local plugins use string names. Allowlisted NPM plugins use an object with 'name', 'package', and optional 'version'. If not provided, only Axe will be performed"
6868
required: false
6969
dry_run:
7070
description: 'When true, scan and log the issues that would be filed without opening, closing, reopening, or assigning any issues, and without writing to the cache.'

0 commit comments

Comments
 (0)