fix: compat with emotion v11.14
#3054
Merged
+556
−166
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
emmenko
requested review from
stephsprinkle,
jaikamat,
ddouglasz,
tylermorrisford,
ByronDWall and
misama-ct
and removed request for
a team
🦋 Changeset detectedLatest commit: 2133ee4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 98 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
emmenko
commented
Feb 5, 2025
Comment on lines
+564
to
+567
| AsyncCreatableSelectInput.ClearIndicator = | ||
| customizedComponents.ClearIndicator as ComponentType< | ||
| ClearIndicatorProps<{}, false, GroupBase<{}>> | ||
| >; |
There was a problem hiding this comment.
These changes are related to the way the bundler option attempts to optimize to avoid long paths.
The error was something like this:
The inferred type of 'AsyncCreatableSelectInput' cannot be named without a reference to '../../../../../../../node_modules/react-select/dist/declarations/src/components/Menu'. This is likely not portable. A type annotation is necessary.
I asked Gemini:
The error arises because TypeScript's `moduleResolution: bundler` (and similar modern module resolution strategies) prioritize bundle size and efficient code splitting. It's trying to avoid emitting very long, specific paths in your type declarations, which could bloat the output and cause issues in different build environments.
The core issue is that TypeScript is inferring a type that directly references an internal, implementation detail of `react-select`. This is fragile because that path could change in future versions of react-select. Your type declarations become tied to the internal structure of a dependency.
The most direct and usually best solution is to explicitly provide a type annotation [...].
Therefore, we are explicitly typing all those "re-exports".
PS: I'm not sure if it still makes sense to keep these "re-exports". We might want to remove them at some point and the consumer would need to import them from react-select directly.
There was a problem hiding this comment.
react-select types are consistently an issue, so it's no real surprise that the re-exports are causing module resolution issues.
The 're-exporting' is happening because of the 'custom' components from uikit that are replacing a subset of the 'default' components from react-select. The 'custom' components should be exported here, since they cannot come from react-select.
There is not a distinction between which components are 'custom' or 'default' in the uikit docs, and only exporting the 'custom' subset seems like it would only make it more confusing to understand how to implement an already fairly complex component.
ddouglasz
approved these changes
Feb 7, 2025
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When upgrading to Emotion
v11.14.0, we get some type errors from the generated declaration files about importsimport("@emotion/react/types/jsx-namespace").EmotionJSX.Element.I reported the issue: emotion-js/emotion#3310.
Then I tried looking at using different options of the
moduleResolutionoption. It seems that the "better" choice would be to use thebundleroption, which actually might be more appropriate anyway for a component library.I went ahead and asked Gemini for some clarification: