Skip to content

Conversation

@david-crespo
Copy link
Collaborator

@david-crespo david-crespo commented Feb 17, 2025

Redoing #2636 without needing to use overrides in package.json by updating some deps and fixing the react peer deps in @oxide/design-system (oxidecomputer/design-system#118) and @oxide/react-asciidoc (oxidecomputer/react-asciidoc#32).

Most of our type errors are due to ReactElement's props generic changing from any to unknown. See https://react.dev/blog/2024/04/25/react-19-upgrade-guide#changes-to-the-reactelement-typescript-type. This is clearly more correct, and forces us to fix some things we were doing unsafely, but it is annoying.

Effect on bundle size

A little bigger. I think a small increase is expected, but I'm not sure exactly how much. I plan to mitigate this anyway very soon by switching to RRv7 framework mode, which automatically code-splits every route.

Before

dist/assets/TimeSeriesChart-D0wiHnXi.js                     392.84 kB │ gzip: 109.67 kB │ map: 1,761.75 kB
dist/assets/app-wGdXIBJe.js                               1,264.09 kB │ gzip: 387.14 kB │ map: 5,705.80 kB

After

dist/assets/TimeSeriesChart-YcjbkudG.js                     406.77 kB │ gzip: 112.08 kB │ map: 1,796.65 kB
dist/assets/app-BwyBpAsf.js                               1,296.62 kB │ gzip: 397.32 kB │ map: 6,200.91 kB

@vercel
Copy link

vercel bot commented Feb 17, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
console ✅ Ready (Inspect) Visit Preview Feb 18, 2025 6:36pm

+ element.style.pointerEvents = 'none';
return element;
}
const $3db38b7d1fb3fe6a$export$be92b6f5f03c0fe9 = $3db38b7d1fb3fe6a$export$ac5b58043b79449b;
Copy link
Collaborator Author

@david-crespo david-crespo Feb 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch isn't needed anymore because we now depend on a version of this library that incorporated our patch.

$ npm ls @radix-ui/react-focus-guards
oxide@0.0.0 /Users/david/oxide/console
└─┬ @radix-ui/react-dialog@1.1.6
  └── @radix-ui/react-focus-guards@1.1.1

-RemoveScroll.classNames = {
- fullWidth: fullWidthClassName,
- zeroRight: zeroRightClassName,
-};
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This existing bit of the patch wasn't necessary. These lines do not set classes, they set a value RemoveScroll.classNames that is imported by other things and used for stuff.

@david-crespo
Copy link
Collaborator Author

The Invalid prop data-headlessui-state supplied to React.Fragment thing is a Headless UI bug that they would love to fix if there was a repro, so I made a repro: tailwindlabs/headlessui#3351 (comment)

<form className="flex">
<Listbox
className="z-10 w-[10rem] border-r border-r-default [&>button]:!rounded-r-none [&>button]:!border-r-0"
className="z-10 w-[10rem] border-r border-r-default [&_button]:!rounded-r-none [&_button]:!border-r-0"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was needed to fix the border between the Listbox and the button after adding a wrapping div to work around the Headless bug. We can always change it back if want after they fix it.

image

// work at the same time (so that, for example, in theory, a button could be
// simultaneously disabled with a tooltip *and* be focused programmatically [I
// tested this]), we merge the two refs inside Tooltip, using child.props.ref to
// get the original ref on the button.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

important comment added

Copy link
Contributor

@charliepark charliepark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of this is looking good to me locally

@david-crespo david-crespo merged commit ba3ab97 into main Feb 18, 2025
7 checks passed
@david-crespo david-crespo deleted the react-19-again-again branch February 18, 2025 23:28
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.

3 participants