diff --git a/src/parser.js b/src/parser.js index d94aa9b..7412af8 100644 --- a/src/parser.js +++ b/src/parser.js @@ -132,7 +132,11 @@ class Parser { return undefined; } const brandLower = brandName.toLowerCase(); - const brand = this._hints.brands.find( + // `Utils.find`, not `Array.prototype.find`: the latter is ES6, and `es5.js` + // ships no polyfills, so it throws outright on the browsers that bundle + // targets. Every other lookup in this file already goes through the helper. + const brand = Utils.find( + this._hints.brands, b => b.brand && b.brand.toLowerCase() === brandLower, ); return brand ? brand.version : undefined; diff --git a/test/acceptance/test-es5-runtime.js b/test/acceptance/test-es5-runtime.js index 3305b35..00d42aa 100644 --- a/test/acceptance/test-es5-runtime.js +++ b/test/acceptance/test-es5-runtime.js @@ -77,6 +77,51 @@ test('the ES5 sandbox actually strips the modern APIs', (t) => { }); }); +/** + * Client Hints go down a different code path than `parse()` — `isBrandVersion` + * and `getBrandVersion` reach into `_hints.brands` directly — so the checks + * above never touch them. `getBrandVersion` used `Array.prototype.find`, which + * is ES6, and threw on exactly the browsers `es5.js` exists for. + * + * The inputs are built by a script evaluated *inside* the context rather than + * assigned onto it. An array created in the host realm keeps the host's + * `Array.prototype`, so its `find` survives the sandbox's delete and the test + * passes against a bug that is still there. A real browser hands the parser a + * same-realm array, which is what this reproduces. + */ +['es5.js', 'bundled.js'].forEach((file) => { + test(`${file} handles Client Hints on an ES5-only global`, (t) => { + const context = createEs5Context(); + vm.runInContext(fs.readFileSync(path.join(root, file), 'utf8'), context); + + const result = vm.runInContext(` + var ua = ${JSON.stringify(UA)}; + var hints = { + brands: [ + { brand: 'Chromium', version: '131' }, + { brand: 'Google Chrome', version: '131' }, + ], + mobile: false, + platform: 'macOS', + }; + var parser = this.bowser.getParser(ua, false, hints); + ({ + brandVersion: parser.getBrandVersion('Google Chrome'), + missingBrand: parser.getBrandVersion('Firefox'), + hasBrand: parser.hasBrand('Google Chrome'), + hasOtherBrand: parser.hasBrand('Firefox'), + hints: !!parser.getHints(), + }) + `, context); + + t.is(result.brandVersion, '131'); + t.is(result.missingBrand, undefined); + t.true(result.hasBrand); + t.false(result.hasOtherBrand); + t.true(result.hints); + }); +}); + test('bundled.js installs the polyfills it promises', (t) => { // The README tells consumers to reach for bundled.js when they have no // polyfills of their own, so it has to actually populate the environment. diff --git a/test/types/check.mjs b/test/types/check.mjs index 7b4c515..b532e72 100644 --- a/test/types/check.mjs +++ b/test/types/check.mjs @@ -2,7 +2,12 @@ * Type-checks a real consumer against the *packed* package under every module * resolution mode TypeScript offers. * - * node test/types/check.mjs + * node test/types/check.mjs [path-to-bowser-x.y.z.tgz] + * + * With a tarball, the published artifact itself is checked — that is how CI + * runs it. With no argument the package is assembled from the working tree's + * `files` allowlist instead, so `pnpm test:types` works from a plain checkout + * without the caller having to pack first. * * `attw` already runs in CI, but it answers a narrower question: whether the * types *resolve* to the right file for each condition. It does not compile @@ -15,9 +20,11 @@ * named exports because importing them that way throws at runtime, and only a * compile that is expected to *fail* can hold that line. * - * Run against the tarball rather than the repo so the exports map, the - * `types`/`typesVersions` fields and the published file list are all exercised - * exactly as a consumer sees them. + * Either way the package is installed into a real `node_modules` and resolved + * by the normal upward walk, so the exports map, the `types` field and the + * published file list are all exercised exactly as a consumer sees them. A + * tsconfig `paths` mapping would resolve the directory directly and silently + * bypass the exports map, which is the thing most worth testing here. */ import cp from 'node:child_process'; import fs from 'node:fs'; @@ -30,8 +37,8 @@ const repoRoot = path.join(here, '..', '..'); const tsc = path.join(repoRoot, 'node_modules', '.bin', 'tsc'); const tarball = process.argv[2]; -if (!tarball || !fs.existsSync(tarball)) { - console.error('usage: node test/types/check.mjs '); +if (tarball && !fs.existsSync(tarball)) { + console.error(`no such tarball: ${tarball}`); process.exit(1); } @@ -60,8 +67,40 @@ const MUST_NOT_COMPILE = [ const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'bowser-types-')); const modules = path.join(tmp, 'node_modules'); fs.mkdirSync(modules, { recursive: true }); -cp.execFileSync('tar', ['xzf', path.resolve(tarball), '-C', modules]); -fs.renameSync(path.join(modules, 'package'), path.join(modules, 'bowser')); + +/** + * Assembles the package from the repo the way `npm pack` would, for when no + * tarball is supplied. `npm pack` is not an option here: package.json carries + * no `version` (it is stamped at release time), and npm refuses to pack + * without one. Copying the `files` allowlist gives the same tree, so + * `pnpm test:types` works from a plain checkout while CI keeps passing the + * real tarball it already builds. + */ +function assembleFromRepo(dest) { + const manifest = JSON.parse(fs.readFileSync(path.join(repoRoot, 'package.json'), 'utf8')); + // npm always includes these regardless of the `files` allowlist. + const entries = [...manifest.files, 'package.json', 'README.md', 'LICENSE']; + for (const entry of entries) { + const from = path.join(repoRoot, entry); + if (!fs.existsSync(from)) continue; + fs.cpSync(from, path.join(dest, entry), { recursive: true }); + } + const missing = ['es5.js', 'bundled.js', 'bowser.mjs'] + .filter((f) => !fs.existsSync(path.join(dest, f))); + if (missing.length) { + console.error(`missing build output: ${missing.join(', ')} — run \`pnpm build\` first`); + process.exit(1); + } +} + +const pkgDir = path.join(modules, 'bowser'); +if (tarball) { + cp.execFileSync('tar', ['xzf', path.resolve(tarball), '-C', modules]); + fs.renameSync(path.join(modules, 'package'), pkgDir); +} else { + fs.mkdirSync(pkgDir, { recursive: true }); + assembleFromRepo(pkgDir); +} /** Runs tsc over a single file in a project configured for `mode`. */ function typeCheck(mode, fileName, source) { @@ -99,7 +138,7 @@ function hash(s) { } let failures = 0; -console.log(`Type-checking consumers against ${path.basename(tarball)}\n`); +console.log(`Type-checking consumers against ${tarball ? path.basename(tarball) : 'the working tree'}\n`); for (const mode of MODES) { const source = fs.readFileSync(path.join(here, 'fixtures', mode.fixture), 'utf8');