Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the ability to extend esbuild options #51

Merged
merged 5 commits into from
Aug 17, 2022

Conversation

zjpalasz
Copy link
Contributor

@zjpalasz zjpalasz commented Jul 18, 2022

From reading some other issues, it sounds like you may be planning to remove esbuild all together. However, in the meantime, I keep running into scenarios that require some specific esbuild options (such as external). This PR allows you to add to the default options:

adapter: azure({
	esbuildOptions: {
		external: [
			'pg-native'
		]
	}
})

@geoffrich
Copy link
Owner

Thanks for the PR! Are there esbuild options you need to set besides external? If possible, I'd prefer to only expose external to minimize the API surface of the adapter.

Please also update the README with the new adapter configuration option.

@zjpalasz
Copy link
Contributor Author

zjpalasz commented Jul 21, 2022

No problem. The two I've come across so far are external and keepNames.

Would you prefer it typed to just those two?

type Options = {
	debug?: boolean;
	customStaticWebAppConfig?: CustomStaticWebAppConfig;
	esbuildOptions?: Pick<esbuild.BuildOptions, 'external' | 'keepNames'>;
};

@geoffrich
Copy link
Owner

Would you prefer it typed to just those two?

Yep, only have those in the typings, and only pass those additional options to esbuild instead of spreading the whole object.

Can you also open an issue with the use cases this is solving for you? This will help me in the future to understand why those options are exposed if I ever remove/replace esbuild.

Adding esbuild options to readme.
@zjpalasz
Copy link
Contributor Author

zjpalasz commented Aug 1, 2022

Fixes #56

@zjpalasz
Copy link
Contributor Author

zjpalasz commented Aug 1, 2022

Can you also open an issue with the use cases this is solving for you? This will help me in the future to understand why those options are exposed if I ever remove/replace esbuild.

I've added the use case for both options. I did see there have been some changes with the azure blob sdk, since I first ran into the need for keepNames. However, I haven't had a chance to see if it resolves the issue. If you prefer, I can reduce this PR to just exclude until I get a chance to confirm whether keepNames is still needed or not?

@geoffrich
Copy link
Owner

Thanks for opening the issue. Yes, please update this PR to only add exclude for now and open a separate PR for keepNames if you confirm it is still needed.

@geoffrich
Copy link
Owner

This is released in 0.9.0. Thanks for your patience!

@zjpalasz zjpalasz deleted the add-esbuild-options branch August 18, 2022 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants