Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.Sign up
Support Pointer Events #12507
This PR adds support for Pointer Events as discussed in #499. It is heavily based on previous work in #1389 and will add most pointer events to the
I added a new DOM fixture to test all pointer events and make sure my implementation does indeed work. I tested on Chrome 65 and Firefox 59 without seeing any issues. If you think the fixtures is not necessary for future changes, I'm happy to remove them as well.
The only open question is if we want to add a polyfill. For the sake of simplicity, I opted against a polyfill for this PR. However, this work is compatible with PEP (I've verified this behavior in Safari 11 by loading PEP in
added a commit
this pull request
Apr 11, 2018
Details of bundled changes.
No more adding any events? Or polyfill? I think react DOM has a responsibility to support the new things in the platform as much as it has one to web perf. Not mention adding new events without bundle increases will likely require a rewrite of the event system. The latter is going to take a long time and involve a lot of input across renderers and FB in general, I hate for RD to fall behind in platform support in the meantime.
I’m not saying to never add them. But this is adding a lot.
I think that might be an overstatement. To drop the 1K pre-gzip, we need to find a way to drop the 1K pre-gzip. Not necessarily rewrite the event system. It could be composed of small improvements anywhere.
We get a lot of PRs with features. I appreciate this but I think it's time more people start hunting for opportunities to cut the size if we want to ship those features.
I tend to start by running
Here are some examples of me doing it: #11912 #11800 #10803 #10802 #10671 #10227. There was no specific pattern other than I looked through all the bundle code, found dubious logic, and deleted or refactored it.
To clarify the previous comment. I don’t mean we need to find a way to add events without increasing the whitelist right now (although that would be nice). I mean that if we want to add something, we need to start being more proactive at removing something. Even if those things are not directly related. This applies both to the core team and to the outside contributions.
Sorry, it's an overstatement if the idea is we should find anything to remove. I do think this PR illustrates that long term, the current system is not great, if we don't want to increase the bundle size as the platform grows. I'd love to start making incremental improvements to the events, (as well as @nhunzaker tho we've both been short on time :/ ). I would say tho, that our explorations there, indicate tho that incremental improvements are hard there, as the whole system sort of neatly rests on itself. Maybe on that front we could do another small call and debrief a bit out it and chart a way forward.
Ya that's great, We should and definitely start building an environment of trimming down the code. I'm just a bit worried about the added outside cost. This adds an even steeper hill to outside contributors, in already hard-to-contribute-to code base. Finding things that are safe to delete requires a lot of experience in the codebase, and as a point of contribution, it's a lot harder to get reviews for those sort of changes. I totally understand the sentiment i just want to make sure we aren't pushing folks away from submitting good additions, because they can't cut an equal amount out somewhere.
Agreed! Rewriting it is a very ambitious goal that also has downstream effects (e.g.
I’m always happy to review bundle size improvements outside of the normal queue. I agree it's not easy but it's necessary.
referenced this pull request
Apr 17, 2018
I'm uncomfortable adding non-core features that increase size at the same time as we're working on core features that will also increase the size.
It's not that there's any particular change or a size limit, but the fact that we get very few pull requests that remove code. So every non-core feature contributes to a death by a thousands cuts.
There was a lot of hard work to get the bundle size down with 16 (partially to “free the space” for new features) and the bar for taking unplanned changes that increase the size is becoming higher now. That can be mitigated with proactive work on getting it down again, but it requires some investment.
I tested the using the DOM fixture against the following browsers to make sure everything works as expected:
However I did find an issue while testing with PEP in Safari 11. According to MDN,
I assume that proper support would need code added in
I think it's a better tradeoff to not fully support PEP for now, hence I've removed the mention of PEP in the parallel guide article (reactjs/reactjs.org#792). It's still possible to use PEP by adding custom event handler to the DOM node. Maybe this is something we want to mention in the docs?