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

Drop support for preact #896

Merged
merged 8 commits into from
Oct 12, 2018
Merged

Drop support for preact #896

merged 8 commits into from
Oct 12, 2018

Conversation

emmatown
Copy link
Member

@emmatown emmatown commented Oct 5, 2018

What:
Remove all the preact packages and stuff in the build setup for it.

Why:
Supporting preact is getting impractical, it was easy in the past when there were very few differences in the react and preact APIs but as react has added new APIs like createContext, forwardRef and Fragment, it's getting significantly harder. While there are implementations of the new context API for preact, they aren't as performant as React's createContext and they can't have some of the upcoming features like unstable_read and static contextType = MyContext.(before anyone says preact is implementing x and y, this isn't just about the current APIs that are missing in preact, it's about future APIs that we couldn't fully utilize because we would still have to support preact) Also, preact-emotion has significantly fewer downloads (~1k vs ~150k downloads a week) than react-emotion so the low demand for it doesn't justify the cost of maintaining it.

Note that the emotion package will still be usable with preact and if someone wants to maintain a separate implementation of styled for preact, I'd be happy to give up the preact-emotion name on npm.

How:

Checklist:

  • Documentation N/A
  • Tests N/A
  • Code complete

@codecov
Copy link

codecov bot commented Oct 8, 2018

Codecov Report

Merging #896 into master will increase coverage by 0.34%.
The diff coverage is 75%.

Impacted Files Coverage Δ
packages/core/src/context.js 100% <100%> (ø) ⬆️
packages/styled-base/src/index.js 98.66% <66.66%> (-0.06%) ⬇️
...-plugin-emotion/src/utils/get-target-class-name.js 100% <0%> (ø) ⬆️

Copy link
Member

@Ailrun Ailrun left a comment

Choose a reason for hiding this comment

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

// preact
for: true,
class: true,
autofocus: true

is also for Preact, isn't it?

@Andarist
Copy link
Member

@Ailrun right, but at the moment is-prop-valid package is also used by other libraries (i.e. styled-components) and they expect preact support, considering that those are just 3 props, I would be in favour of keeping them here

@emmatown
Copy link
Member Author

I agree with @Andarist.

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