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

Problems with tree-shaking (forwardRef|displayName) #3126

Open
Kiyozz opened this issue Sep 19, 2024 · 0 comments
Open

Problems with tree-shaking (forwardRef|displayName) #3126

Kiyozz opened this issue Sep 19, 2024 · 0 comments

Comments

@Kiyozz
Copy link

Kiyozz commented Sep 19, 2024

Bug report

Current Behavior

Tree-shaking is not optimized/not working.

Current components are not using PURE annotation or is using displayName that cause tree-shaking problems.

Current Dialog component

const DialogOverlay = React.forwardRef<DialogOverlayElement, DialogOverlayProps>(
  (props: ScopedProps<DialogOverlayProps>, forwardedRef) => {
    const portalContext = usePortalContext(OVERLAY_NAME, props.__scopeDialog);
    const { forceMount = portalContext.forceMount, ...overlayProps } = props;
    const context = useDialogContext(OVERLAY_NAME, props.__scopeDialog);
    return context.modal ? (
      <Presence present={forceMount || context.open}>
        <DialogOverlayImpl {...overlayProps} ref={forwardedRef} />
      </Presence>
    ) : null;
  }
);

Easy first solution to do

// Make it PURE and keep comments in the bundle
// bundlers understand and keep PURE annotations anyway from my experience
const DialogOverlay = /* @__PURE__ */ React.forwardRef<DialogOverlayElement, DialogOverlayProps>(
  (props: ScopedProps<DialogOverlayProps>, forwardedRef) => {
    const portalContext = usePortalContext(OVERLAY_NAME, props.__scopeDialog);
    const { forceMount = portalContext.forceMount, ...overlayProps } = props;
    const context = useDialogContext(OVERLAY_NAME, props.__scopeDialog);
    return context.modal ? (
      <Presence present={forceMount || context.open}>
        <DialogOverlayImpl {...overlayProps} ref={forwardedRef} />
      </Presence>
    ) : null;
  }
);

Expected behavior

Unused components should be removed from the bundle.

Reproducible example

Repository

Suggested solution

  • Make forwardRef components pure with a comment /* @__PURE__ */ forwardRef(...).
  • Make Component.displayName = conditional for components to be removed.

Additional context

Issue #336 with PR #577 fixed tree-shaking in 2021. However, this was reverted later, and I don't know why.

In my repository, after bundling the UI and the Vite App (See the README), you can see the Dialogs reference (DialogPortal for example) in the final bundle because esbuild cannot understand the Dialog could be removed because there is no PURE annotations or displayName is used.

Your environment

CleanShot 2024-09-19 at 14 23 16@2x

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

No branches or pull requests

1 participant