[@nx/esbuild:esbuild] Give precedence to config from esbuildOptions or esbuildConfig over executor options.

[sharky98]

• I'd be willing to implement this feature (contributing guide)

Description

Using a custom configuration either from using esbuildOptions or esbuildConfig does not work for all esbuild options. For example, esbuild allows for iife format¹, while the executor options does not. Since the executor format default to ["esm"], even not providing it will override esbuildOptions or esbuildConfig.

I would like the behavior of the executor to use esbuildOptions or esbuildConfig in priority over the executor options (obviously, when it does not prevent the good execution of the executor itself²).

Motivation

While the executor options exposes most esbuild own options, they are not one-to-one matches. Some (edge?) cases (such as using iife format or using a esbuild plugins that requires write: false) may need to deviate from the allowed executor options.

Moreover, it would allow to support options that esbuild would create in the future without having to write code to expose them explicitly.

Suggested Implementation

On the end-user side, I would expect no changes in configuration format. However, it would be a breaking changes that the esbuildOptions or esbuildConfig options take precedence over executor options.

Code-wise, here my two cents how I would see it done. However I do not know very well Nx ins and outs, so there are probably some cascading changes to be made also. But this is the gist of my idea.

• Move the buildEsbuildOptions() spread operator on options.userDefinedBuildOptions at the end of the object so that it overrides any options created by the executor.
• If there are options that must be controlled by the executor and never by esbuild for cases like commonality between all bundler executor, etc. (entryNames, external, sourcemap seems to be like that due to how they implement some decision factor based on executor options), they could be move after the spread operator. However, in that case, I think the executor should expose a slimmed down type of esbuild.BuildOptions that remove/changes the allowed options.

nx/packages/esbuild/src/executors/esbuild/lib/build-esbuild-options.ts

Lines 25 to 49 in 7942ea3

	const esbuildOptions: esbuild.BuildOptions = {
	...options.userDefinedBuildOptions,
	entryNames:
	options.outputHashing === 'all' ? '[dir]/[name].[hash]' : '[dir]/[name]',
	bundle: options.bundle,
	// Cannot use external with bundle option
	external: options.bundle
	? [
	...(options.userDefinedBuildOptions?.external ?? []),
	...options.external,
	]
	: undefined,
	minify: options.minify,
	platform: options.platform,
	target: options.target,
	metafile: options.metafile,
	tsconfig: options.tsConfig,
	sourcemap:
	(options.sourcemap ?? options.userDefinedBuildOptions?.sourcemap) ||
	false,
	format,
	outExtension: {
	'.js': outExtension,
	},
	};

Other few ideas that could be useful in the refactoring.

• Remove the format argument from buildEsbuildOptions() and uses the one provided in NormalizedEsBuildExecutorOptions.format (an array) or NormalizedEsBuildExecutorOptions.userDefinedBuildOptions.format (a single item).
• outExtension and other functions that use limited configuration from what esbuild allows will probably need some gymnastics to be made more generic (and still valid!).
• Have buildEsbuildOptions() to return an array of esbuild.BuildOptions based on the requested format. That also mean, having to change to loop on the built options instead of the provided format in the implementation of both watch and build
• Foolproof the yield of the executor if write: false is provided as a custom option². Either have Nx write the files or have the outfile empty (and let whatever uses that field to not fail, since having write: false means the use was deliberate in its decision).

Alternate Implementations

• Alternatively, the executor could be expanded and reworked to allow current esbuild options; and updated each time esbuild add new options.

Footnotes

1. iife format is in fact the default esbuild format when bundling: true and platform: "browser"; two options exposed by the executor options. ↩

2. For example, I tried to use write: false, which failed because the executor expect the files to be created when it yield. The outfile field seems to be populated, but since no files are created by esbuild whatever uses that field fail later on. Some plugins relies on write: false to either write themselves or expect the user to write the file themselves after the build process. ↩ ↩²

[github-actions[bot]]

This issue has been automatically marked as stale because it hasn't had any activity for 6 months.
Many things may have changed within this time. The issue may have already been fixed or it may not be relevant anymore.
If at this point, this is still an issue, please respond with updated information.
It will be closed in 21 days if no further activity occurs.
Thanks for being a part of the Nx community! 🙏

[Coly010]

Hi! thanks for the detailed explanation, it is very much appreciated.

I converted the issue to a discussion so that some design can be hashed out and allow others to chime in such that we could potentially implement this in the most non-breaking method possible