-
-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
Build: Fix the sb-bench CI step #19029
Conversation
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.
You are the king!!! 👑
Emotion changed to use exports map, and esbuild wasn't pivking the right entry anymore, resulting in react not being detected, and thus not bundled, and thus missing at runtime! I changed the tsup/esbuild config to prefer the ESM version, and this seems to have fixed the issue. Many thanks to @anderist for the assist!
…the suggested fix by @anderist He suggested that 'hiding' the dependency on an export that may or may not exists away from bundlers would solve it. This is what they do in emotion itself, but esbuild optimizes that away again.
@@ -80,6 +80,7 @@ const run = async ({ cwd, flags }: { cwd: string; flags: string[] }) => { | |||
: false, | |||
esbuildOptions: (c) => { | |||
/* eslint-disable no-param-reassign */ | |||
c.conditions = ['module']; |
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.
I would recheck what kind of an effect does it have on esbuild - is this purely additive to the default conditions? or does it replace them? I think that I was checking this in the past and that this is additive - some default conditions just cant be turned off but I'm not 100% sure.
So it seems there's a bunch of these missing / broken: |
# Conflicts: # code/yarn.lock
Issue: The CI task of sb-bench was failing.
related: #18992 I think this replaces it. @Andarist