fix(module): refuse to add a config key a spread or computed key could shadow

Author: danielroe

packages/nuxt-cli/src/utils/config-parse.ts

@@ -151,6 +151,8 @@ function locateWithParser(parseSync: ParseSync, source: string, filename: string
 thrownewUnknownAstError('the config object does not hold ESTree properties')
 }
 
+constdynamic=object.properties.some((property: any)=>property.type==='SpreadElement'||property.computed===true)
+
 constpropertyStarts=object.properties.map((property: any)=>property.start)
 constkeys={}asRecord<ConfigKey,ArrayLocation>
 
@@ -165,6 +167,7 @@ function locateWithParser(parseSync: ParseSync, source: string, filename: string
 objectStart: object.start,
  propertyStarts,
  keys,
+ dynamic,
 quote: detectQuote(source,allElements(keys)),
 }
 }
@@ -287,7 +290,7 @@ function locateWithScan(source: string): ConfigLocation {
 thrownewConfigShapeError('Default export is missing in the config file!')
 }
 
-constproperties=scanProperties(source,objectStart)
+const{properties, complete }=scanProperties(source,objectStart)
 constkeys={}asRecord<ConfigKey,ArrayLocation>
 
 for(constkeyofCONFIG_KEYS){
@@ -299,6 +302,7 @@ function locateWithScan(source: string): ConfigLocation {
  objectStart,
 propertyStarts: properties.map(property=>property.start),
  keys,
+dynamic: !complete,
 quote: detectQuote(source,allElements(keys)),
 }
 }
@@ -357,30 +361,34 @@ interface ScannedProperty {
 valueStart: number
 }
 
-functionscanProperties(source: string,objectStart: number): ScannedProperty[]{
+functionscanProperties(source: string,objectStart: number): {properties: ScannedProperty[],complete: boolean}{
 constobjectEnd=matchBracket(source,objectStart)
 if(objectEnd===undefined){
 thrownewConfigShapeError('The config object is not terminated.')
 }
 
 constproperties: ScannedProperty[]=[]
+letcomplete=true
 letat=skipTrivia(source,objectStart+1)
 
 while(at<objectEnd){
 conststart=at
 constkey=readKey(source,at)
 if(!key){
+complete=false
 break
 }
 at=skipTrivia(source,key.end)
 if(source[at]!==':'){
+complete=false
 break
 }
 constvalueStart=at+1
 properties.push({key: key.value, start, valueStart })
 
 constvalueEnd=skipValue(source,valueStart,objectEnd)
 if(valueEnd===undefined){
+complete=false
 break
 }
 at=skipTrivia(source,valueEnd)
@@ -389,7 +397,7 @@ function scanProperties(source: string, objectStart: number): ScannedProperty[]
 }
 }
 
-returnproperties
+return{properties, complete }
 }
 
 functionreadKey(source: string,at: number): {value: string,end: number}|undefined{
@@ -566,6 +574,12 @@ export interface ConfigLocation {
 /** Offsets of each property key in the exported object, used to match indentation. */
 propertyStarts: number[]
 keys: Record<ConfigKey,ArrayLocation>
+/**
+ * Whether the object holds something that could define a key we cannot see: a
+ * spread, a computed key, or (for the scanner) a property it could not read.
+ * Adding a key such a config may already set would silently shadow it.
+ */
+dynamic: boolean
 /** Quote character to use for new entries. */
 quote: string
 }

packages/nuxt-cli/src/utils/config.ts

@@ -108,6 +108,9 @@ export async function addNuxtConfigEntries(config: NuxtConfigFile, entries: Conf
 edits.push(buildInsert(source,location,array,names))
 }
 else{
+if(location.dynamic){
+thrownewActionableError(`Could not add \`${key}\` to ${config.file}: the config spreads or computes keys, so a new \`${key}\` could be silently overridden. Add ${names.map(name=>`\`${name}\``).join(', ')} to \`${key}\` by hand.`)
+}
 created.push(buildProperty(source,location,key,names))
 }
 }

packages/nuxt-cli/test/unit/utils/config-dynamic.spec.ts

@@ -0,0 +1,109 @@
+import{mkdir,mkdtemp,readFile,realpath,symlink,writeFile}from'node:fs/promises'
+import{tmpdir}from'node:os'
+import{fileURLToPath}from'node:url'
+
+import{dirname,join}from'pathe'
+import{afterAll,afterEach,describe,expect,it,vi}from'vitest'
+
+import{addNuxtConfigEntries,readNuxtConfig}from'../../../src/utils/config'
+
+constrolldownPath=dirname(fileURLToPath(import.meta.resolve('rolldown/package.json')))
+
+constdirectories: string[]=[]
+
+afterEach(()=>{
+vi.unstubAllEnvs()
+})
+
+afterAll(async()=>{
+const{ rm }=awaitimport('node:fs/promises')
+awaitPromise.all(directories.splice(0).map(directory=>rm(directory,{recursive: true,force: true})))
+})
+
+asyncfunctioncreateProject(source: string,{ parser }: {parser: boolean}): Promise<string>{
+constcwd=awaitrealpath(awaitmkdtemp(join(tmpdir(),'nuxi-config-dynamic-')))
+directories.push(cwd)
+awaitwriteFile(join(cwd,'nuxt.config.ts'),source,'utf8')
+if(parser){
+awaitmkdir(join(cwd,'node_modules'),{recursive: true})
+awaitsymlink(rolldownPath,join(cwd,'node_modules/rolldown'),'dir')
+}
+else{
+vi.stubEnv('NUXT_CLI_PARSER','scanner')
+}
+returncwd
+}
+
+describe.each([
+['scanner',{parser: false}],
+['parser',{parser: true}],
+])('%s',(name,options)=>{
+constrefusal=name==='parser' ? 'Could not find a config object' : 'Default export is missing'
+asyncfunctionadd(source: string): Promise<{error?: string,after: string}>{
+constcwd=awaitcreateProject(source,options)
+constconfig=awaitreadNuxtConfig(cwd)
+leterror: string|undefined
+try{
+awaitaddNuxtConfigEntries(config!,{modules: ['@nuxt/image']})
+}
+catch(err){
+error=(errasError).message
+}
+return{ error,after: awaitreadFile(join(cwd,'nuxt.config.ts'),'utf8')}
+}
+
+it('should refuse to add a key to a config that spreads another object',async()=>{
+constsource='export default defineNuxtConfig({ ...base })\n'
+
+const{ error, after }=awaitadd(source)
+
+expect(error).toContain('silently overridden')
+expect(after).toBe(source)
+})
+
+it('should never end up with two `modules` keys next to a spread',async()=>{
+const{ error, after }=awaitadd('export default defineNuxtConfig({ ...base, modules: [\'a\'] })\n')
+
+expect(after.match(/modules:/g)).toHaveLength(1)
+if(error){
+expect(error).toContain('silently overridden')
+}
+else{
+expect(after).toContain('modules: [\'a\', \'@nuxt/image\']')
+}
+})
+
+it('should refuse to add a key next to a computed one',async()=>{
+constsource='const key = \'modules\'\nexport default defineNuxtConfig({ [key]: [\'a\'] })\n'
+
+const{ error, after }=awaitadd(source)
+
+expect(error).toContain('silently overridden')
+expect(after).toBe(source)
+})
+
+it('should refuse a config whose default export is not an object',async()=>{
+constcwd=awaitcreateProject('export default defineNuxtConfig(base)\n',options)
+
+awaitexpect(readNuxtConfig(cwd)).rejects.toThrow(refusal)
+})
+
+it('should refuse a config chosen by a conditional',async()=>{
+constcwd=awaitcreateProject('export default defineNuxtConfig(x ? { modules: [] } : { modules: [\'a\'] })\n',options)
+
+awaitexpect(readNuxtConfig(cwd)).rejects.toThrow(refusal)
+})
+
+it('should refuse a config returned from a function',async()=>{
+constcwd=awaitcreateProject('export default defineNuxtConfig(() => ({ modules: [] }))\n',options)
+
+awaitexpect(readNuxtConfig(cwd)).rejects.toThrow(refusal)
+})
+
+it('should add a key to an ordinary config',async()=>{
+const{ error, after }=awaitadd('export default defineNuxtConfig({ ssr: false })\n')
+
+expect(error).toBeUndefined()
+expect(after).toContain('@nuxt/image')
+})
+})