fix(browser): don't exclude node builtins from optimization (#5082)

Author: sheremet-va

packages/browser/src/node/esmInjector.ts

@@ -38,8 +38,6 @@ export function injectVitestModule(code: string, id: string, parse: PluginContex
 
   const hoistIndex = 0
 
-  let hasInjected = false
-
   // this will transform import statements into dynamic ones, if there are imports
   // it will keep the import as is, if we don't need to mock anything
   // in browser environment it will wrap the module value with "vitest_wrap_module" function
@@ -76,7 +74,6 @@ export function injectVitestModule(code: string, id: string, parse: PluginContex
   }
 
   function defineExport(position: number, name: string, local = name) {
-    hasInjected = true
     s.appendLeft(
       position,
       `\nObject.defineProperty(${viInjectedKey}, "${name}", `
@@ -170,7 +167,6 @@ export function injectVitestModule(code: string, id: string, parse: PluginContex
         // named hoistable/class exports
         // export default function foo() {}
         // export default class A {}
-        hasInjected = true
         const { name } = node.declaration.id
         s.remove(node.start, node.start + 15 /* 'export default '.length */)
         s.append(
@@ -180,7 +176,6 @@ export function injectVitestModule(code: string, id: string, parse: PluginContex
       }
       else {
         // anonymous default exports
-        hasInjected = true
         s.update(
           node.start,
           node.start + 14 /* 'export default'.length */,
@@ -196,13 +191,10 @@ export function injectVitestModule(code: string, id: string, parse: PluginContex
       s.remove(node.start, node.end)
       const importId = defineImportAll(node.source.value as string)
       // hoist re-exports near the defined import so they are immediately exported
-      if (node.exported) {
+      if (node.exported)
         defineExport(hoistIndex, node.exported.name, `${importId}`)
-      }
-      else {
-        hasInjected = true
+      else
         s.appendLeft(hoistIndex, `${viExportAllHelper}(${viInjectedKey}, ${importId});\n`)
-      }
     }
   }
 
@@ -244,11 +236,10 @@ export function injectVitestModule(code: string, id: string, parse: PluginContex
     },
   })
 
-  if (hasInjected) {
-    // make sure "__vi_injected__" is declared as soon as possible
-    s.prepend(`const ${viInjectedKey} = { [Symbol.toStringTag]: "Module" };\n`)
-    s.append(`\nexport { ${viInjectedKey} }`)
-  }
+  // make sure "__vi_injected__" is declared as soon as possible
+  // prepend even if file doesn't export anything
+  s.prepend(`const ${viInjectedKey} = { [Symbol.toStringTag]: "Module" };\n`)
+  s.append(`\nexport { ${viInjectedKey} }`)
 
   return {
     ast,

packages/browser/src/node/index.ts

@@ -1,7 +1,6 @@
 import { fileURLToPath } from 'node:url'
 
 import { resolve } from 'node:path'
-import { builtinModules } from 'node:module'
 import sirv from 'sirv'
 import type { Plugin } from 'vite'
 import type { WorkspaceProject } from 'vitest/node'
@@ -61,18 +60,19 @@ export default (project: WorkspaceProject, base = '/'): Plugin[] => {
               'vitest/runners',
             ],
             exclude: [
-              ...builtinModules,
               'vitest',
               'vitest/utils',
               'vitest/browser',
               'vitest/runners',
               '@vitest/utils',
+
+              // loupe is manually transformed
+              'loupe',
             ],
             include: [
               'vitest > @vitest/utils > pretty-format',
               'vitest > @vitest/snapshot > pretty-format',
               'vitest > diff-sequences',
-              'vitest > loupe',
               'vitest > pretty-format',
               'vitest > pretty-format > ansi-styles',
               'vitest > pretty-format > ansi-regex',
@@ -81,6 +81,13 @@ export default (project: WorkspaceProject, base = '/'): Plugin[] => {
           },
         }
       },
+      transform(code, id) {
+        if (id.includes('loupe/loupe.js')) {
+          const exportsList = ['custom', 'inspect', 'registerConstructor', 'registerStringTag']
+          const codeAppend = exportsList.map(i => `export const ${i} = globalThis.loupe.${i}`).join('\n')
+          return `${code}\n${codeAppend}\nexport default globalThis.loupe`
+        }
+      },
       async resolveId(id) {
         if (!/\?browserv=\w+$/.test(id))
           return

packages/utils/src/display.ts

@@ -1,6 +1,6 @@
 // since this is already part of Vitest via Chai, we can just reuse it without increasing the size of bundle
 // @ts-expect-error doesn't have types
-import { inspect as loupe } from 'loupe'
+import * as loupe from 'loupe'
 
 interface LoupeOptions {
   showHidden?: boolean | undefined
@@ -101,7 +101,7 @@ export function format(...args: unknown[]) {
 export function inspect(obj: unknown, options: LoupeOptions = {}): string {
   if (options.truncate === 0)
     options.truncate = Number.POSITIVE_INFINITY
-  return loupe(obj, options)
+  return loupe.inspect(obj, options)
 }
 
 export function objDisplay(obj: unknown, options: LoupeOptions = {}): string {

pnpm-lock.yaml

@@ -16,6 +16,7 @@
     "@vitest/cjs-lib": "link:./cjs-lib",
     "execa": "^7.1.1",
     "safaridriver": "^0.0.4",
+    "url": "^0.11.3",
     "vitest": "workspace:*"
   }
 }

test/browser/package.json

@@ -20,7 +20,7 @@ export default async function runVitest(moreArgs = []) {
   const browserResult = await readFile('./browser.json', 'utf-8')
   const browserResultJson = JSON.parse(browserResult)
 
-  const getPassed = results => results.filter(result => result.status === 'passed')
+  const getPassed = results => results.filter(result => result.status === 'passed' && !result.mesage)
   const getFailed = results => results.filter(result => result.status === 'failed')
 
   const passedTests = getPassed(browserResultJson.testResults)

test/browser/specs/run-vitest.mjs

@@ -11,10 +11,11 @@ const {
 } = await runVitest()
 
 await test('tests are actually running', async () => {
-  assert.ok(browserResultJson.testResults.length === 12, 'Not all the tests have been run')
-  assert.ok(passedTests.length === 10, 'Some tests failed')
-  assert.ok(failedTests.length === 2, 'Some tests have passed but should fail')
+  assert.equal(browserResultJson.testResults.length, 14, 'Not all the tests have been run')
+  assert.equal(passedTests.length, 12, 'Some tests failed')
+  assert.equal(failedTests.length, 2, 'Some tests have passed but should fail')
 
+  assert.doesNotMatch(stderr, /has been externalized for browser compatibility/, 'doesn\'t have any externalized modules')
   assert.doesNotMatch(stderr, /Unhandled Error/, 'doesn\'t have any unhandled errors')
 })
 

test/browser/specs/runner.test.mjs

@@ -0,0 +1,7 @@
+// eslint-disable-next-line unicorn/prefer-node-protocol
+import * as url from 'url'
+import { expect, test } from 'vitest'
+
+test('url is polifylled because it\'s installed in dependencies', () => {
+  expect(url.format).toBeDefined()
+})

test/browser/test/polyfill-lib.test.ts

@@ -0,0 +1,6 @@
+import { inspect } from 'vitest/utils'
+import { expect, it } from 'vitest'
+
+it('utils package correctly uses loupe', async () => {
+  expect(inspect({ test: 1 })).toBe('{ test: 1 }')
+})