metadata returned by plugin is inconsistent happens

[sapphi-red]

Maybe Related: #2176, vitejs/vite#7683, vitejs/vite#7685

Reproduction

1. Open https://stackblitz.com/edit/node-d2wvrw?file=bundle.js
2. Download it
3. Run npm i and node bundle.js

On windows, Detected inconsistent metadata for the path "src/c.js" when it was imported here: happens.
But on stackblitz, it does not happen.

Full error log

X [ERROR] Detected inconsistent metadata for the path "src/c.js" when it was imported here:

    src/b.js:1:7:
      1 │ import './c.js';
        ╵        ~~~~~~~~

  The original metadata for that path comes from when it was imported here:

    src/a.js:1:7:
      1 │ import '@/c.js';
        ╵        ~~~~~~~~

  The difference in metadata is displayed below:

   {
  -  "pluginName": "alias",
  -  "type": null,
  +  "pluginName": null,
  +  "type": "module",
   }

  This is a bug in the "alias" plugin. Plugins provide metadata for a given path in an "onResolve"
  callback. All metadata provided for the same path must be consistent to ensure deterministic
  builds. Due to parallelism, one set of provided metadata will be randomly chosen for a given path,       
  so providing inconsistent metadata for the same path can cause non-determinism.

C:\Users\green\Downloads\node-d2wvrw\node_modules\esbuild\lib\main.js:1603
  let error = new Error(`${text}${summary}`);
              ^

Error: Build failed with 1 error:
src/b.js:1:7: ERROR: Detected inconsistent metadata for the path "src/c.js" when it was imported here:     
    at failureErrorWithLog (C:\Users\green\Downloads\node-d2wvrw\node_modules\esbuild\lib\main.js:1603:15) 
    at C:\Users\green\Downloads\node-d2wvrw\node_modules\esbuild\lib\main.js:1249:28
    at runOnEndCallbacks (C:\Users\green\Downloads\node-d2wvrw\node_modules\esbuild\lib\main.js:1034:63)   
    at buildResponseToResult (C:\Users\green\Downloads\node-d2wvrw\node_modules\esbuild\lib\main.js:1247:7)    at C:\Users\green\Downloads\node-d2wvrw\node_modules\esbuild\lib\main.js:1356:14
    at C:\Users\green\Downloads\node-d2wvrw\node_modules\esbuild\lib\main.js:666:9
    at handleIncomingPacket (C:\Users\green\Downloads\node-d2wvrw\node_modules\esbuild\lib\main.js:763:9)  
    at Socket.readFromStdout (C:\Users\green\Downloads\node-d2wvrw\node_modules\esbuild\lib\main.js:632:7) 
    at Socket.emit (node:events:526:28)
    at addChunk (node:internal/streams/readable:315:12) {
  errors: [
    {
      detail: undefined,
      location: {
        column: 7,
        file: 'src/b.js',
        length: 8,
        line: 1,
        lineText: "import './c.js';",
        namespace: '',
        suggestion: ''
      },
      notes: [
        {
          location: {
            column: 7,
            file: 'src/a.js',
            length: 8,
            line: 1,
            lineText: "import '@/c.js';",
            namespace: '',
            suggestion: ''
          },
          text: 'The original metadata for that path comes from when it was imported here:'
        },
        {
          location: null,
          text: 'The difference in metadata is displayed below:'
        },
        { location: null, text: '' },
        { location: null, text: ' {' },
        { location: null, text: '-  "pluginName": "alias",' },
        { location: null, text: '-  "type": null,' },
        { location: null, text: '+  "pluginName": null,' },
        { location: null, text: '+  "type": "module",' },
        { location: null, text: ' }' },
        { location: null, text: '' },
        {
          location: null,
          text: 'This is a bug in the "alias" plugin. Plugins provide metadata for a given path in an "onResolve" callback. All metadata provided for the same path must be consistent to ensure deterministic builds. Due to parallelism, one set of provided metadata will be randomly chosen for a given path, so providing inconsistent metadata for the same path can cause non-determinism.'
        }
      ],
      pluginName: '',
      text: 'Detected inconsistent metadata for the path "src/c.js" when it was imported here:'
    }
  ],
  warnings: []
}

[sapphi-red]

This also happens with https://stackblitz.com/edit/node-dmisef?file=bundle.js.

plugin of first reproduction

const aliasPlugin = {
  name: 'alias',
  setup({ onResolve }) {
    onResolve({ filter: /^@/ }, ({ path: p, importer }) => {
      return {
        path: path.resolve(path.dirname(importer), p.replace(/^@/, '.')),
      };
    });
  },
};

plugin of second reproduction

const aliasPlugin = {
  name: 'alias',
  setup({ onResolve, resolve }) {
    onResolve({ filter: /^@/ }, async ({ path: p, importer }) => {
      return await resolve(p.replace(/^@/, '.'), {
        resolveDir: path.dirname(importer),
      });
    });
  },
};

[acidjazz]

Definitely happing locally here on OSX

[alex-4ed5ec60d]

can reproduce on Linux as well

[sapphi-red]

I confirmed this happens with linux by using docker.
Maybe it cannot be reprocuced only with stackblitz.

[sapphi-red]

The diff of "type": "module", was happening because finalizeResolve is not called when it is resolved by a plugin.

esbuild/internal/bundler/bundler.go

Lines 788 to 794 in 757d7d9

	return &resolver.ResolveResult{
	PathPair: resolver.PathPair{Primary: result.Path},
	IsExternal: result.External,
	PluginName: pluginName,
	PluginData: result.PluginData,
	PrimarySideEffectsData: sideEffectsData,
	}, false, resolver.DebugMeta{}

esbuild/internal/resolver/resolver.go

Lines 654 to 782 in 757d7d9

	func (r resolverQuery) finalizeResolve(result *ResolveResult) {
	if !result.IsExternal && r.isExternal(r.options.ExternalSettings.PostResolve, result.PathPair.Primary.Text) {
	if r.debugLogs != nil {
	r.debugLogs.addNote(fmt.Sprintf("The path %q was marked as external by the user", result.PathPair.Primary.Text))
	}
	result.IsExternal = true
	return
	}
	for _, path := range result.PathPair.iter() {
	if path.Namespace == "file" {
	if dirInfo := r.dirInfoCached(r.fs.Dir(path.Text)); dirInfo != nil {
	base := r.fs.Base(path.Text)
	// Look up this file in the "sideEffects" map in the nearest enclosing
	// directory with a "package.json" file.
	//
	// Only do this for the primary path. Some packages have the primary
	// path marked as having side effects and the secondary path marked
	// as not having side effects. This is likely a bug in the package
	// definition but we don't want to consider the primary path as not
	// having side effects just because the secondary path is marked as
	// not having side effects.
	if pkgJSON := dirInfo.enclosingPackageJSON; pkgJSON != nil && *path == result.PathPair.Primary {
	if pkgJSON.sideEffectsMap != nil {
	hasSideEffects := false
	if pkgJSON.sideEffectsMap[path.Text] {
	// Fast path: map lookup
	hasSideEffects = true
	} else {
	// Slow path: glob tests
	for _, re := range pkgJSON.sideEffectsRegexps {
	if re.MatchString(path.Text) {
	hasSideEffects = true
	break
	}
	}
	}
	if !hasSideEffects {
	if r.debugLogs != nil {
	r.debugLogs.addNote(fmt.Sprintf("Marking this file as having no side effects due to %q",
	pkgJSON.source.KeyPath.Text))
	}
	result.PrimarySideEffectsData = pkgJSON.sideEffectsData
	}
	}
	// Also copy over the "type" field
	result.ModuleTypeData = pkgJSON.moduleTypeData
	}
	// Copy various fields from the nearest enclosing "tsconfig.json" file if present
	if path == &result.PathPair.Primary && dirInfo.enclosingTSConfigJSON != nil {
	// Except don't do this if we're inside a "node_modules" directory. Package
	// authors often publish their "tsconfig.json" files to npm because of
	// npm's default-include publishing model and because these authors
	// probably don't know about ".npmignore" files.
	//
	// People trying to use these packages with esbuild have historically
	// complained that esbuild is respecting "tsconfig.json" in these cases.
	// The assumption is that the package author published these files by
	// accident.
	//
	// Ignoring "tsconfig.json" files inside "node_modules" directories breaks
	// the use case of publishing TypeScript code and having it be transpiled
	// for you, but that's the uncommon case and likely doesn't work with
	// many other tools anyway. So now these files are ignored.
	if helpers.IsInsideNodeModules(result.PathPair.Primary.Text) {
	if r.debugLogs != nil {
	r.debugLogs.addNote(fmt.Sprintf("Ignoring %q because %q is inside \"node_modules\"",
	dirInfo.enclosingTSConfigJSON.AbsPath,
	result.PathPair.Primary.Text))
	}
	} else {
	result.JSXFactory = dirInfo.enclosingTSConfigJSON.JSXFactory
	result.JSXFragment = dirInfo.enclosingTSConfigJSON.JSXFragmentFactory
	result.UseDefineForClassFieldsTS = dirInfo.enclosingTSConfigJSON.UseDefineForClassFields
	result.UnusedImportsTS = config.UnusedImportsFromTsconfigValues(
	dirInfo.enclosingTSConfigJSON.PreserveImportsNotUsedAsValues,
	dirInfo.enclosingTSConfigJSON.PreserveValueImports,
	)
	result.TSTarget = dirInfo.enclosingTSConfigJSON.TSTarget
	if r.debugLogs != nil {
	r.debugLogs.addNote(fmt.Sprintf("This import is under the effect of %q",
	dirInfo.enclosingTSConfigJSON.AbsPath))
	if result.JSXFactory != nil {
	r.debugLogs.addNote(fmt.Sprintf("\"jsxFactory\" is %q due to %q",
	strings.Join(result.JSXFactory, "."),
	dirInfo.enclosingTSConfigJSON.AbsPath))
	}
	if result.JSXFragment != nil {
	r.debugLogs.addNote(fmt.Sprintf("\"jsxFragment\" is %q due to %q",
	strings.Join(result.JSXFragment, "."),
	dirInfo.enclosingTSConfigJSON.AbsPath))
	}
	}
	}
	}
	if !r.options.PreserveSymlinks {
	if entry, _ := dirInfo.entries.Get(base); entry != nil {
	if symlink := entry.Symlink(r.fs); symlink != "" {
	// Is this entry itself a symlink?
	if r.debugLogs != nil {
	r.debugLogs.addNote(fmt.Sprintf("Resolved symlink %q to %q", path.Text, symlink))
	}
	path.Text = symlink
	} else if dirInfo.absRealPath != "" {
	// Is there at least one parent directory with a symlink?
	symlink := r.fs.Join(dirInfo.absRealPath, base)
	if r.debugLogs != nil {
	r.debugLogs.addNote(fmt.Sprintf("Resolved symlink %q to %q", path.Text, symlink))
	}
	path.Text = symlink
	}
	}
	}
	}
	}
	}
	if r.debugLogs != nil {
	r.debugLogs.addNote(fmt.Sprintf("Primary path is %q in namespace %q", result.PathPair.Primary.Text, result.PathPair.Primary.Namespace))
	if result.PathPair.HasSecondary() {
	r.debugLogs.addNote(fmt.Sprintf("Secondary path is %q in namespace %q", result.PathPair.Secondary.Text, result.PathPair.Secondary.Namespace))
	}
	}
	}

[evanw]

Yes, I can confirm locally as well. Thanks for the report and for the suggested fix. This clearly should have been introduced as a warning instead of an error though. My apologies.

I believe what esbuild is flagging here may be representative of a real issue that was previously undiscovered. The discrepancy between the module types could hypothetically lead to a divergence in build behavior in this case depending on which set of metadata is chosen first. I will revert this for now and then work on getting this fixed.

[evanw]

The revert has been published as 0.14.36, so that version should work. Sorry about the breakage.

A bit more background: As a performance shortcut, esbuild's internals pass metadata about the resolved file (e.g. module and sideEffects from package.json) from the path resolution phase directly to the parsing phase instead of re-discovering it when parsing a file. That way it doesn't need to be looked up again. There isn't a concern about non-determinism when esbuild does this because path resolution always runs the same code and so is always consistent.

However, when the plugin API was added that design wasn't changed, and is error-prone as a result because plugins can inject arbitrary code that doesn't follow the original rules. I could make the suggested change above to call finalizeResolve but that would overwrite these metadata values from onResolve plugins and make it not possible to set metadata via plugins anymore. Instead I could consider making a breaking change to the plugin API to move this metadata into either onLoad or into another plugin callback to decorate a file with metadata. This wouldn't be error-prone because it would only happen once per file, but it would be slower because it would involve more communication overhead and delays in the pipeline.