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

chore: Bump Preact version #490

Merged
merged 5 commits into from
Oct 7, 2024
Merged

chore: Bump Preact version #490

merged 5 commits into from
Oct 7, 2024

Conversation

rschristian
Copy link
Member

Devtools is failing on the ecosystem-ci I had set up.

Two issues:

  • preactjs/preact#4173 altered the structure of preact/src/internal to wrap all of the interfaces in declare global, so we can no longer access them directly
    • I see you've vendored some of the types for v11 here, but I went with patch-package for the moment. Happy to go with whatever you prefer, I absolutely can copy the types here if you'd like, import and use those types as globals, etc., just thought I'd throw out this approach first as it's the simplest and probably most straightforward.
  • preactjs/preact#3690 improved the types for bare h() calls, so a couple of the tests where you add arbitrary props (foo, __source) now fail type checks.

@rschristian rschristian marked this pull request as draft October 4, 2024 02:53
@rschristian
Copy link
Member Author

rschristian commented Oct 4, 2024

Didn't realize yarn test didn't cover all tests. Will have to look into it more.

Edit: Ah, falling prey to the (breaking) change in data- attributes & something else (or a few things). Bigger change than I thought, will take some time.

@@ -165,7 +165,7 @@
z-index: 2;
}

.icon-btn:not([disabled])[data-active] .icon-btn-inner,
.icon-btn:not([disabled])[data-active="true"] .icon-btn-inner,
Copy link
Member Author

Choose a reason for hiding this comment

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

Helpful ripgrep pattern: $ rg 'data-[\w-]+]'

@rschristian
Copy link
Member Author

Should be good to go. It was indeed the breaking change in data attributes & the addition of .contains

@rschristian rschristian marked this pull request as ready for review October 4, 2024 06:12
@rschristian
Copy link
Member Author

@marvinhagemeister Just FYI, I'm not on the authorized users list here so I cannot merge.

Absolutely 0 rush to land this, I've switch the CI over to use this branch anyhow so it'll catch any future changes in Preact that may affect this repo, just wanted to mention it in case the intention was that I could merge whenever I thought it was good.

@JoviDeCroock JoviDeCroock merged commit e74468f into main Oct 7, 2024
1 check passed
@JoviDeCroock JoviDeCroock deleted the chore/bump-preact branch October 7, 2024 06:49
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