Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 39 additions & 3 deletions app/components/Package/InstallScripts.vue
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,37 @@ import { getOutdatedTooltip, getVersionClass } from '~/utils/npm/outdated-depend

const props = defineProps<{
packageName: string
version: string
installScripts: {
scripts: ('preinstall' | 'install' | 'postinstall')[]
content?: Record<string, string>
npxDependencies: Record<string, string>
}
}>()

function getCodeLink(filePath: string): string {
return `/code/${props.packageName}/v/${props.version}/${filePath}`
}

const scriptParts = computed(() => {
const parts: Record<string, { prefix: string | null; filePath: string | null; link: string }> = {}
for (const scriptName of props.installScripts.scripts) {
const content = props.installScripts.content?.[scriptName]
if (!content) continue
const parsed = parseNodeScript(content)
if (parsed) {
parts[scriptName] = {
prefix: parsed.prefix,
filePath: parsed.filePath,
link: getCodeLink(parsed.filePath),
}
} else {
parts[scriptName] = { prefix: null, filePath: null, link: getCodeLink('package.json') }
}
}
return parts
})

const outdatedNpxDeps = useOutdatedDependencies(() => props.installScripts.npxDependencies)
const hasNpxDeps = computed(() => Object.keys(props.installScripts.npxDependencies).length > 0)
const sortedNpxDeps = computed(() => {
Expand All @@ -30,11 +54,23 @@ const isExpanded = shallowRef(false)
<div v-for="scriptName in installScripts.scripts" :key="scriptName">
<dt class="font-mono text-xs text-fg-muted">{{ scriptName }}</dt>
<dd
tabindex="0"
class="font-mono text-sm text-fg-subtle m-0 truncate focus:whitespace-normal focus:overflow-visible cursor-help rounded focus-visible:(outline-2 outline-accent outline-offset-2)"
class="font-mono text-sm text-fg-subtle m-0 truncate"
:title="installScripts.content?.[scriptName]"
>
{{ installScripts.content?.[scriptName] || $t('package.install_scripts.script_label') }}
<template v-if="installScripts.content?.[scriptName] && scriptParts[scriptName]">
<template v-if="scriptParts[scriptName].prefix">
{{ scriptParts[scriptName].prefix
}}<LinkBase :to="scriptParts[scriptName].link">{{
scriptParts[scriptName].filePath
}}</LinkBase>
</template>
<LinkBase v-else :to="scriptParts[scriptName].link">
{{ installScripts.content[scriptName] }}
</LinkBase>
</template>
<span v-else tabindex="0" class="cursor-help">
{{ $t('package.install_scripts.script_label') }}
</span>
</dd>
</div>
</dl>
Expand Down
1 change: 1 addition & 0 deletions app/pages/package/[[org]]/[name].vue
Original file line number Diff line number Diff line change
Expand Up @@ -1170,6 +1170,7 @@ onKeyStroke(
<PackageInstallScripts
v-if="displayVersion?.installScripts"
:package-name="pkg.name"
:version="displayVersion.version"
:install-scripts="displayVersion.installScripts"
/>

Expand Down
64 changes: 64 additions & 0 deletions app/utils/install-scripts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,67 @@ export function extractInstallScriptsInfo(
npxDependencies: extractNpxDependencies(scripts),
}
}

/**
* Pattern to match scripts that are just `node <file-path>`
* Captures the file path (relative paths with alphanumeric chars, dots, hyphens, underscores, and slashes)
*/
const NODE_SCRIPT_PATTERN = /^node\s+([\w./-]+)$/

/**
* Get the file path for an install script link.
* - If the script is `node <file-path>`, returns that file path
* - Otherwise, returns 'package.json'
*
* @param scriptContent - The content of the script
* @returns The file path to link to in the code tab
*/
export function getInstallScriptFilePath(scriptContent: string): string {
const match = NODE_SCRIPT_PATTERN.exec(scriptContent)

if (match?.[1]) {
// Script is `node <file-path>`, link to that file
// Normalize path: strip leading ./
const filePath = match[1].replace(/^\.\//, '')

// Fall back to package.json if path contains navigational elements (the client-side routing can't handle these well)
if (filePath.includes('../') || filePath.includes('./')) {
return 'package.json'
}
Comment on lines +135 to +143
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Reject absolute paths to avoid broken code‑tab links.
Absolute paths (e.g., node /usr/bin/install.js) currently match and will be linked, even though they won’t exist in the package code. Treat absolute paths as non‑linkable and fall back to package.json/null.

🛠️ Suggested fix
-    if (filePath.includes('../') || filePath.includes('./')) {
+    if (
+      filePath.startsWith('/') ||
+      filePath.includes('../') ||
+      filePath.includes('./')
+    ) {
       return 'package.json'
     }
-    if (filePath.includes('../') || filePath.includes('./')) {
+    if (
+      filePath.startsWith('/') ||
+      filePath.includes('../') ||
+      filePath.includes('./')
+    ) {
       return null
     }

Also applies to: 169-172


return filePath
}

// Default: link to package.json
return 'package.json'
}

/**
* Parse an install script into a prefix and a linkable file path.
* - If the script is `node <file-path>`, returns { prefix: 'node ', filePath: '<file-path>' }
* so only the file path portion can be rendered as a link.
* - Otherwise, returns null (the entire script content should link to package.json).
*
* @param scriptContent - The content of the script
* @returns Parsed parts, or null if no node file path was extracted
*/
export function parseNodeScript(
scriptContent: string,
): { prefix: string; filePath: string } | null {
const match = NODE_SCRIPT_PATTERN.exec(scriptContent)

if (match?.[1]) {
const filePath = match[1].replace(/^\.\//, '')

// Fall back if path contains navigational elements
if (filePath.includes('../') || filePath.includes('./')) {
return null
}

// Reconstruct the prefix (everything before the captured file path)
const prefix = scriptContent.slice(0, match.index + match[0].indexOf(match[1]))
return { prefix, filePath }
Comment on lines +174 to +176
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Prefix extraction breaks when the file name is node.
Using indexOf(match[1]) returns 0 for node node, producing an empty prefix and dropping the node part in the UI. Derive the prefix by length instead.

🛠️ Suggested fix
-    const prefix = scriptContent.slice(0, match.index + match[0].indexOf(match[1]))
+    const prefix = scriptContent.slice(0, scriptContent.length - filePath.length)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Reconstruct the prefix (everything before the captured file path)
const prefix = scriptContent.slice(0, match.index + match[0].indexOf(match[1]))
return { prefix, filePath }
// Reconstruct the prefix (everything before the captured file path)
const prefix = scriptContent.slice(0, scriptContent.length - filePath.length)
return { prefix, filePath }

}

return null
}
2 changes: 2 additions & 0 deletions test/nuxt/a11y.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1993,6 +1993,7 @@ describe('component accessibility audits', () => {
const component = await mountSuspended(PackageInstallScripts, {
props: {
packageName: 'esbuild',
version: '0.25.0',
installScripts: {
scripts: ['postinstall'],
content: { postinstall: 'node install.js' },
Expand All @@ -2008,6 +2009,7 @@ describe('component accessibility audits', () => {
const component = await mountSuspended(PackageInstallScripts, {
props: {
packageName: 'husky',
version: '9.1.0',
installScripts: {
scripts: ['postinstall'],
content: { postinstall: 'husky install' },
Expand Down
59 changes: 58 additions & 1 deletion test/unit/app/utils/install-scripts.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { describe, expect, it } from 'vitest'
import { extractInstallScriptsInfo } from '../../../../app/utils/install-scripts'
import {
extractInstallScriptsInfo,
getInstallScriptFilePath,
parseNodeScript,
} from '../../../../app/utils/install-scripts'

describe('extractInstallScriptsInfo', () => {
it('returns null when no install scripts exist', () => {
Expand Down Expand Up @@ -75,3 +79,56 @@ describe('extractInstallScriptsInfo', () => {
})
})
})

describe('getInstallScriptFilePath', () => {
it('returns file path when script is `node <file-path>`', () => {
expect(getInstallScriptFilePath('node scripts/postinstall.js')).toBe('scripts/postinstall.js')
})

it('returns package.json when script is not a simple node command', () => {
expect(getInstallScriptFilePath('npx prisma generate')).toBe('package.json')
})

it('strips leading ./ from relative paths', () => {
expect(getInstallScriptFilePath('node ./scripts/setup.js')).toBe('scripts/setup.js')
})

it('falls back to package.json for parent directory references', () => {
expect(getInstallScriptFilePath('node ../scripts/setup.js')).toBe('package.json')
expect(getInstallScriptFilePath('node ./scripts/../lib/setup.js')).toBe('package.json')
})

it('returns package.json for bare node command without arguments', () => {
expect(getInstallScriptFilePath('node')).toBe('package.json')
expect(getInstallScriptFilePath('node ')).toBe('package.json')
})
})

describe('parseNodeScript', () => {
it('returns prefix and filePath for node scripts', () => {
expect(parseNodeScript('node scripts/postinstall.js')).toEqual({
prefix: 'node ',
filePath: 'scripts/postinstall.js',
})
})

it('strips leading ./ from file path', () => {
expect(parseNodeScript('node ./scripts/setup.js')).toEqual({
prefix: 'node ',
filePath: 'scripts/setup.js',
})
})

it('returns null for non-node scripts', () => {
expect(parseNodeScript('npx prisma generate')).toBeNull()
})

it('returns null for bare node command', () => {
expect(parseNodeScript('node')).toBeNull()
expect(parseNodeScript('node ')).toBeNull()
})

it('returns null for parent directory references', () => {
expect(parseNodeScript('node ../scripts/setup.js')).toBeNull()
})
})
Loading