perf: stop build plugins doing work the bundler can filter (#871)

Author: harlan-zw

packages/script/src/module.ts

@@ -828,9 +828,11 @@ export default defineNuxtModule<ModuleOptions>({
 
 constmoduleInstallPromises: Map<string,()=>Promise<boolean>|undefined>=newMap()
 
-addBuildPlugin(NuxtScriptsCheckScripts(),{
-dev: true,
-})
+// Only guards against `await $script` in dev. `addBuildPlugin`'s `dev` option cannot
+// express "dev builds only": it skips on `dev: false`, and `nuxt.options.build` is
+// always truthy, so the check has to happen here.
+if(nuxt.options.dev)
+addBuildPlugin(NuxtScriptsCheckScripts())
 addBuildPlugin(NuxtScriptBundleTransformer({
  nuxt,
 scripts: registryScriptsWithImport,

packages/script/src/plugins/check-scripts.ts

@@ -3,7 +3,10 @@ import { parseAndWalk } from 'oxc-walker'
 import{createUnplugin}from'unplugin'
 import{isVue}from'./util'
 
-constVUE_RE=/\.vue/
+constVUE_RE=/\.vue(?:\?|$)/
+// `$script` is only reachable through a `useScript` call, so the bundler can skip
+// every other module before the hook runs.
+constUSE_SCRIPT_CODE_MARKER='useScript'
 
 exportfunctionNuxtScriptsCheckScripts(){
 returncreateUnplugin(()=>{
@@ -12,12 +15,11 @@ export function NuxtScriptsCheckScripts() {
 transform: {
 filter: {
 id: VUE_RE,
+code: USE_SCRIPT_CODE_MARKER,
 },
 handler(code,id){
 if(!isVue(id,{type: ['script']}))
 return
-if(!code.includes('useScript'))// all integrations should start with useScript*
-return
 
 letnameNode: Node|undefined
 leterrorNode: Node|undefined

packages/script/src/plugins/transform.ts

@@ -18,14 +18,19 @@ import { bundleStorage } from '../assets'
 import{logger}from'../logger'
 import{getBundleResolve}from'../registry'
 import{rewriteScriptUrlsAST}from'./rewrite-ast'
-import{isJS,isVue}from'./util'
+import{isVue}from'./util'
 
 constSEVEN_DAYS_IN_MS=7*24*60*60*1000
 
 constPROTOCOL_RELATIVE_RE=/^\/\//
-constVUE_RE=/\.vue/
-constJS_RE=/\.[cm]?[jt]sx?$/
+// Ids carry a query in dev and for SFC blocks, so every extension match allows one.
+constVUE_RE=/\.vue(?:\?|$)/
+constJS_RE=/\.[cm]?[jt]sx?(?:\?|$)/
 constTEST_RE=/\.(?:test|spec)\./
+// Every integration is called through `useScript` or `useScriptX`, so a module without
+// that substring can never need this transform. The bundler applies it, natively where
+// it can, so the hook is not called at all for the rest of the graph.
+constUSE_SCRIPT_CODE_MARKER='useScript'
 constUPPERCASE_RE=/^[A-Z]$/
 constUSE_SCRIPT_RE=/^useScript/
 
@@ -261,11 +266,11 @@ export function NuxtScriptBundleTransformer(options: AssetBundlerTransformerOpti
 include: [VUE_RE,JS_RE],
 exclude: [TEST_RE],
 },
+code: USE_SCRIPT_CODE_MARKER,
 },
 asynchandler(code,id){
-if(!isVue(id,{type: ['template','script']})&&!isJS(id))
-return
-if(!code.includes('useScript'))// all integrations should start with useScriptX
+// A `.vue` id reaches us once per SFC block. Only script and template are ours.
+if(VUE_RE.test(id)&&!isVue(id,{type: ['template','script']}))
 return
 
 consts=newMagicString(code)

packages/script/src/plugins/util.ts

@@ -34,11 +34,3 @@ export function isVue(id: string, opts: { type?: Array<'template' | 'script' | '
 // Query `?vue&type=template` (in Webpack or external template)
 returntrue
 }
-
-constJS_RE=/\.(?:[cm]?j|t)sx?$/
-
-exportfunctionisJS(id: string){
-// JavaScript files
-const{ pathname }=parseURL(decodeURIComponent(pathToFileURL(id).href))
-returnJS_RE.test(pathname)
-}

test/unit/check-scripts.test.ts

@@ -1,19 +1,16 @@
 import{describe,expect,it}from'vitest'
 import{NuxtScriptsCheckScripts}from'../../packages/script/src/plugins/check-scripts'
+import{runTransform}from'../utils/unplugin'
 
 constplugin=NuxtScriptsCheckScripts().vite()asany
 
-asyncfunctiontransform(code: string|string[]){
+asyncfunctiontransform(code: string|string[],id='file.vue'){
 consterrors: Error[]=[]
-awaitplugin.transform.handler.call(
-{
-error: (e: Error)=>{
-errors.push(e)
-},
-},
-Array.isArray(code) ? code.join('\n') : code,
-'file.vue',
-)
+awaitrunTransform(plugin,{
+ id,
+code: Array.isArray(code) ? code.join('\n') : code,
+context: {error: (e: Error)=>{errors.push(e)}},
+})
 returnerrors
 }
 
@@ -107,3 +104,30 @@ const _sfc_main = /* @__PURE__ */ _defineComponent({
 expect(awaittransform(code)).toMatchInlineSnapshot(`[]`)
 })
 })
+
+describe('module scope',()=>{
+// Compiled shape of `await $script` in an SFC: the destructure, then `_withAsyncContext`.
+constoffending=[
+'const { $script } = useScript("/test.js");',
+'let __temp, __restore;',
+'[__temp, __restore] = _withAsyncContext(() => $script), await __temp, __restore();',
+].join('\n')
+
+it.each([
+['file.vue','a bare SFC'],
+['file.vue?vue&type=script&setup=true&lang.ts','an SFC script block'],
+])('inspects %s (%s)',async(id)=>{
+expect(awaittransform(offending,id)).not.toEqual([])
+})
+
+it.each([
+['file.vue?vue&type=style&index=0&lang.css','a style block'],
+['file.ts','a plain module'],
+])('leaves %s alone (%s)',async(id)=>{
+expect(awaittransform(offending,id)).toEqual([])
+})
+
+it('skips a component that never calls useScript',async()=>{
+expect(awaittransform(`const answer = await fetchAnswer()`)).toEqual([])
+})
+})

test/unit/transform.test.ts

@@ -6,6 +6,7 @@ import { hash } from 'ohash'
 import{hasProtocol,joinURL,withBase}from'ufo'
 import{beforeEach,describe,expect,it,vi}from'vitest'
 import{NuxtScriptBundleTransformer}from'../../packages/script/src/plugins/transform'
+import{runTransform}from'../utils/unplugin'
 
 constohash=(awaitvi.importActual<typeofimport('ohash')>('ohash')).hash
 vi.mock('ohash',async(og)=>{
@@ -61,16 +62,18 @@ vi.mocked(hasProtocol).mockImplementation(() => true)
 // hash receive a URL object, we want to mock it to return the pathname by default
 vi.mocked(hash).mockImplementation(src=>src.pathname)
 
-asyncfunctiontransform(code: string|string[],options?: AssetBundlerTransformerOptions){
+asyncfunctiontransformId(id: string,code:string,options?: AssetBundlerTransformerOptions){
 constplugin=NuxtScriptBundleTransformer({ ...options,nuxt: mockNuxt}).vite()asany
-constres=awaitplugin.transform.handler.call(
-{},
-Array.isArray(code) ? code.join('\n') : code,
-'file.js',
-)
+// Goes through the declared filter, so a filter regression fails here rather than silently
+// shipping a plugin that never sees the files it should.
+constres=awaitrunTransform(plugin,{ id, code })
 returnres?.code
 }
 
+asyncfunctiontransform(code: string|string[],options?: AssetBundlerTransformerOptions){
+returntransformId('file.js',Array.isArray(code) ? code.join('\n') : code,options)
+}
+
 describe('nuxtScriptTransformer',()=>{
 it('string arg',async()=>{
 vi.mocked(hash).mockImplementationOnce(()=>'beacon.min')
@@ -1314,4 +1317,29 @@ const _sfc_main = /* @__PURE__ */ _defineComponent({
 expect(code).toContain('bundle.js')
 })
 })
+
+describe('module scope',()=>{
+constbundled=`const instance = useScript('https://example.com/s.js', { bundle: true })`
+
+it.each([
+'file.ts',
+'file.mts',
+'file.cts',
+'file.mjs',
+'file.jsx',
+'file.vue',
+'file.vue?vue&type=script&setup=true&lang.ts',
+'file.ts?t=1699999999999',
+])('transforms %s',async(id)=>{
+vi.mocked(hash).mockImplementationOnce(()=>'s')
+expect(awaittransformId(id,bundled)).toContain('/_scripts/')
+})
+
+it.each([
+'file.vue?vue&type=style&index=0&lang.css',
+'file.vue?nuxt_component=async',
+])('leaves %s alone',async(id)=>{
+expect(awaittransformId(id,bundled)).toBeUndefined()
+})
+})
 })

test/utils/unplugin.ts

@@ -0,0 +1,64 @@
+/**
+ * Mirrors unplugin's `StringFilter`. Declared here because `unplugin` is a dependency of
+ * the module package, not of the test root.
+ */
+typeFilterPattern=string|RegExp|Array<string|RegExp>
+typeStringFilter=FilterPattern|{include?: FilterPattern,exclude?: FilterPattern}
+
+/**
+ * Calling `plugin.transform.handler` directly skips the declared `transform.filter`,
+ * so a filter that stops matching the files it should would fail no test. These helpers
+ * apply the filter first, the way a bundler does.
+ *
+ * Semantics mirror unplugin's `createFilterForTransform`: a string pattern is a
+ * substring test, a RegExp is `test()`, an array is OR, and any `exclude` match vetoes.
+ */
+
+functionmatches(pattern: string|RegExp,value: string): boolean{
+returntypeofpattern==='string' ? value.includes(pattern) : pattern.test(value)
+}
+
+functionmatchesFilter(filter: StringFilter|undefined,value: string): boolean{
+if(filter===undefined)
+returntrue
+if(typeoffilter==='string'||filterinstanceofRegExp)
+returnmatches(filter,value)
+if(Array.isArray(filter))
+returnfilter.some(pattern=>matches(pattern,value))
+
+const{ include, exclude }=filter
+if(exclude!==undefined){
+constexcluded=Array.isArray(exclude)
+ ? exclude.some(pattern=>matches(pattern,value))
+ : matches(exclude,value)
+if(excluded)
+returnfalse
+}
+if(include===undefined)
+returntrue
+returnArray.isArray(include)
+ ? include.some(pattern=>matches(pattern,value))
+ : matches(include,value)
+}
+
+/** Would a bundler hand this module to the plugin's transform hook? */
+exportfunctiontransformAccepts(plugin: any,id: string,code: string): boolean{
+constfilter=plugin.transform?.filter
+if(!filter)
+returntrue
+returnmatchesFilter(filter.id,id)&&matchesFilter(filter.code,code)
+}
+
+/**
+ * Run a transform the way a bundler would: filter, then handler.
+ * Returns `undefined` when the filter rejects the module, matching a hook that never ran.
+ */
+exportasyncfunctionrunTransform(
+plugin: any,
+{ id, code, context ={}}: {id: string,code: string,context?: Record<string,unknown>},
+): Promise<any>{
+if(!transformAccepts(plugin,id,code))
+returnundefined
+consthandler=plugin.transform?.handler??plugin.transform
+returnhandler.call(context,code,id)
+}