-
Notifications
You must be signed in to change notification settings - Fork 40
misc(esbuild): Log warning when attempting to inject debug IDs with esbuild bundle
option active
#526
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
Conversation
…sbuild `bundle` option active
packages/esbuild-plugin/src/index.ts
Outdated
@@ -50,6 +51,12 @@ function esbuildDebugIdInjectionPlugin(): UnpluginOptions { | |||
|
|||
esbuild: { | |||
setup({ initialOptions, onLoad, onResolve }) { | |||
if (initialOptions.bundle) { | |||
logger.warn( | |||
"Esbuild's `bundle` option is currently not supported! Esbuild will probably crash now." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
l: Did I understand the issue correctly that this only happens for bundle: true
? In this case, maybe we tell users this specifically:
"Esbuild's `bundle` option is currently not supported! Esbuild will probably crash now." | |
"Esbuild's `bundle: true` option is currently not supported! Esbuild will probably crash now." |
Also, is there anything we can recommend them here instead of just saying that their build will crash?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely valid point. I updated the wording!
@Lms24 would you mind taking another look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, sorry this fell through. LGTM and thx for making the change!
![]() ![]() I'm confused here cc @lforst |
omg I think I completely flipped what I meant to do in that PR. Gonna fix it on monday. |
Relates to: #445 (comment)