Skip to content

Commit b943f01

Browse files
committed
feat(javascript-web): keep browser projects without a lockfile off the Node SDK
javascript_web requires a committed lockfile and javascript_node matches any package.json, so a browser-only project whose lockfile is not committed falls through to the Node integration and the run installs the server SDK. Add one branch that runs only after the existing lockfile check declines: claim the project when index.html sits in the ROOT and no server framework is declared. Root-only matters — the existing hasIndexHtml helper globs six levels deep, which is fine alongside a lockfile and far too loose when deciding alone; it would pull in a Node library with a docs/index.html. A site with no package.json at all stays unclaimed on purpose — PostHog/posthog.com#18390 routes those users to the JS snippet from the docs, which is the better answer. Refs #870
1 parent a1bcf57 commit b943f01

3 files changed

Lines changed: 123 additions & 2 deletions

File tree

src/frameworks/javascript-web/javascript-web-wizard-agent.ts

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,11 @@ import { hasDeclaredDependency } from '@utils/package-json';
88
import { tryGetPackageJson } from '@utils/setup-utils';
99
import {
1010
FRAMEWORK_PACKAGES,
11+
SERVER_PACKAGES,
1112
detectJsPackageManager,
1213
detectBundler,
1314
hasIndexHtml,
15+
hasRootIndexHtml,
1416
type JavaScriptContext,
1517
} from './utils';
1618
import { detectNodePackageManagers } from '@lib/detection/package-manager';
@@ -38,6 +40,10 @@ export const JAVASCRIPT_WEB_AGENT_CONFIG: FrameworkConfig<JavaScriptContext> = {
3840
detectPackageManager: detectNodePackageManagers,
3941
detect: async (options) => {
4042
const packageJson = await tryGetPackageJson(options);
43+
44+
// A site with no package.json at all is deliberately left unclaimed: the
45+
// snippet is the whole integration and there is nothing for the wizard to
46+
// do. PostHog/posthog.com#18390 routes those users to it from the docs.
4147
if (!packageJson) {
4248
return false;
4349
}
@@ -73,6 +79,18 @@ export const JAVASCRIPT_WEB_AGENT_CONFIG: FrameworkConfig<JavaScriptContext> = {
7379
return true;
7480
}
7581

82+
// No lockfile. `javascriptNode` matches any package.json, so falling
83+
// through here hands a browser-only project the server SDK. Claim it
84+
// instead, but only on evidence strong enough to stand without the
85+
// lockfile: an index.html in the ROOT (the site's entry point, not a
86+
// `docs/` page or a generated report), and no server framework declared.
87+
if (
88+
hasRootIndexHtml(options) &&
89+
!SERVER_PACKAGES.some((pkg) => hasDeclaredDependency(pkg, packageJson))
90+
) {
91+
return true;
92+
}
93+
7694
// Otherwise → Node/Backend (handled by javascriptNode fallback)
7795
return false;
7896
},
@@ -100,9 +118,9 @@ export const JAVASCRIPT_WEB_AGENT_CONFIG: FrameworkConfig<JavaScriptContext> = {
100118

101119
prompts: {
102120
projectTypeDetection:
103-
'This is a JavaScript/TypeScript project. Look for package.json and lockfiles (package-lock.json, yarn.lock, pnpm-lock.yaml, bun.lockb) to confirm.',
121+
'This is a browser-side JavaScript/TypeScript project. Look for package.json and lockfiles (package-lock.json, yarn.lock, pnpm-lock.yaml, bun.lockb) to confirm.',
104122
packageInstallation:
105-
'Look for lockfiles to determine the package manager (npm, yarn, pnpm, bun). Do not manually edit package.json.',
123+
'Look for lockfiles to determine the package manager (npm, yarn, pnpm, bun). A project without a committed lockfile is still a browser project — install posthog-js with the package manager the project appears to use. Do not manually edit package.json.',
106124
getAdditionalContextLines: (context) => {
107125
const lines = [
108126
`Package manager: ${context.packageManagerName ?? 'unknown'}`,

src/frameworks/javascript-web/utils.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,33 @@ export const FRAMEWORK_PACKAGES = [
3232
'@sveltejs/kit',
3333
] as const;
3434

35+
/**
36+
* Packages that mean the project serves requests rather than shipping a browser
37+
* bundle. An index.html sitting next to one of these is a server's static asset
38+
* directory, not a static site, so `javascript_node` should keep the match.
39+
*/
40+
export const SERVER_PACKAGES = [
41+
'express',
42+
'fastify',
43+
'koa',
44+
'@nestjs/core',
45+
'@hapi/hapi',
46+
'hapi',
47+
'restify',
48+
'h3',
49+
] as const;
50+
51+
/**
52+
* An index.html in the project root — the entry point of a site, as opposed to
53+
* a `docs/` page or a generated report deeper in the tree. Used only where a
54+
* match has to stand on its own without a lockfile to corroborate it.
55+
*/
56+
export function hasRootIndexHtml(
57+
options: Pick<WizardRunOptions, 'installDir'>,
58+
): boolean {
59+
return fs.existsSync(path.join(options.installDir, 'index.html'));
60+
}
61+
3562
/**
3663
* Detect the JS package manager for the project by checking lockfiles.
3764
* Reuses the existing package manager detection infrastructure.

src/lib/detection/__tests__/framework.test.ts

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,82 @@ describe('detectFramework (end-to-end over real project dirs)', () => {
115115
);
116116
});
117117

118+
test('a browser project with no committed lockfile is not handed to javascript_node', async () => {
119+
const opts = project({
120+
'package.json': JSON.stringify({
121+
name: 'portfolio',
122+
scripts: { build: 'tailwindcss -o dist/style.css' },
123+
}),
124+
'index.html': '<!doctype html><html></html>',
125+
});
126+
await expect(detectFramework(opts.installDir)).resolves.toBe(
127+
Integration.javascript_web,
128+
);
129+
});
130+
131+
test('an Express server with a root index.html stays on javascript_node', async () => {
132+
const opts = project({
133+
'package.json': JSON.stringify({ dependencies: { express: '^4' } }),
134+
'index.html': '<!doctype html><html></html>',
135+
});
136+
await expect(detectFramework(opts.installDir)).resolves.toBe(
137+
Integration.javascriptNode,
138+
);
139+
});
140+
141+
test('a Node library with a docs/index.html stays on javascript_node', async () => {
142+
const opts = project({
143+
'package.json': JSON.stringify({ name: 'lib' }),
144+
'src/index.js': 'module.exports = {}',
145+
'docs/index.html': '<!doctype html><html></html>',
146+
});
147+
await expect(detectFramework(opts.installDir)).resolves.toBe(
148+
Integration.javascriptNode,
149+
);
150+
});
151+
152+
test('a server framework we do not list is still safe if index.html is not at the root', async () => {
153+
const opts = project({
154+
'package.json': JSON.stringify({ dependencies: { hono: '^4' } }),
155+
'public/index.html': '<!doctype html><html></html>',
156+
});
157+
await expect(detectFramework(opts.installDir)).resolves.toBe(
158+
Integration.javascriptNode,
159+
);
160+
});
161+
162+
test('a lockfile project keeps matching on a nested index.html (unchanged)', async () => {
163+
const opts = project({
164+
'package.json': JSON.stringify({ name: 'site' }),
165+
'package-lock.json': '{}',
166+
'public/index.html': '<!doctype html><html></html>',
167+
});
168+
await expect(detectFramework(opts.installDir)).resolves.toBe(
169+
Integration.javascript_web,
170+
);
171+
});
172+
173+
test('a Node project with no frontend signal stays on javascript_node', async () => {
174+
const opts = project({
175+
'package.json': JSON.stringify({ dependencies: { lodash: '^4' } }),
176+
'index.js': 'module.exports = {}',
177+
});
178+
await expect(detectFramework(opts.installDir)).resolves.toBe(
179+
Integration.javascriptNode,
180+
);
181+
});
182+
183+
// A site with no package.json is left unclaimed on purpose — the snippet is
184+
// the whole integration. PostHog/posthog.com#18390 routes those users to it.
185+
test('a hand-written static site is deliberately claimed by nobody', async () => {
186+
const opts = project({
187+
'index.html': '<!doctype html><html><head></head><body>hi</body></html>',
188+
'about.html': '<!doctype html><html></html>',
189+
'css/style.css': 'body { color: black }',
190+
});
191+
await expect(detectFramework(opts.installDir)).resolves.toBeUndefined();
192+
});
193+
118194
test('a Kotlin Multiplatform project resolves to kmp', async () => {
119195
const opts = project({
120196
'settings.gradle.kts': 'include(":shared")',

0 commit comments

Comments
 (0)