[@types/react] Upgrade types for React 16.4; add Pointer Events support (wip, awaiting feedback)

[lostfictions]

Please fill in this template.

• Use a meaningful title for the pull request. Include the name of the package modified.
• Test the change in your own code. (Compile and run.)
• Add or edit tests to reflect the change. (Run with npm test.)
• Follow the advice from the readme.
• Avoid common mistakes.
• Run npm run lint package-name (or tsc if no tslint.json is present).

Select one of these and delete the others:

If changing an existing definition:

• Provide a URL to documentation or source code which provides context for the suggested changes: https://reactjs.org/blog/2018/05/23/react-v-16-4.html

  (see also the PR on facebook/react to add pointer events support.)

• Increase the version number in the header if appropriate.

• If you are making substantial changes, consider adding a tslint.json containing { "extends": "dtslint/dt.json" }.

[lostfictions]

No tests added since none of the existing React tests are for basic DOM attributes, but I've tested and run these defs against my own code.

(And yes, onGotPointerCaptureCapture and onLostPointerCaptureCapture are not typos.)

[lostfictions]

Looks like the CI failure is due to the defs for react-pointable (...which is deprecated as of React 16.4.)

Unfortunately I'm not sure I have more time to contribute in order to resolve these errors -- maybe @iStefo or @mDibyo who contributed the react-pointable types could have a moment to look into them?

EDIT: took a quick look and it seems like the simplest solution might be to remove the offending lines in the defs for react-pointable:

DefinitelyTyped/types/react-pointable/index.d.ts

Lines 16 to 23 in ebadb72

	onPointerMove?(evt: PointerEvent): void;
	onPointerDown?(evt: PointerEvent): void;
	onPointerUp?(evt: PointerEvent): void;
	onPointerOver?(evt: PointerEvent): void;
	onPointerOut?(evt: PointerEvent): void;
	onPointerEnter?(evt: PointerEvent): void;
	onPointerLeave?(evt: PointerEvent): void;
	onPointerCancel?(evt: PointerEvent): void;

Since PointableProps extends HTMLAttributes and SVGAttributes, those props will still be available and work as normal. I can give this a shot tomorrow.

[iStefo]

I haven't tested your changes, but it looks reasonable.

[mDibyo]

onPointer... props would be available in React.HTMLAttributes only in React 16.4 and above. That means this change would break the react-pointable typings for people using React <16.4. Am I missing something?

[mDibyo]

Looks like it would be possible to declare a dependency to react@<16.4 in @types/react-pointable's package.json. @lostfictions, unless I got something wrong in my previous comment, could you try adding that dependency?

[lostfictions]

@mDibyo I think that's right -- react-pointable users would have to install an older version of the typings. Unfortunately it seems like everything in DT has to be versioned in lockstep, so there's no way to upgrade the react typings without changing the react-pointable ones... maybe @johnnyreilly or another DT maintainer can confirm?

I guess an alternative would be to remove the react-pointable typings altogether (which I assume would leave the existing versions on npm)... but that might leave people who upgrade to react 16.4 but who still have react-pointable in their codebase stuck until they remove it? That may still make more sense, though.

[mDibyo]

@lostfictions Please take a look at my previous comment if you haven't already. We posted the comments at the same time, so not sure if you have seen my comment. :)

Adding a dependency to react@<16.4 would not solve the scenario you mentioned in your comment, namely people who want to use react 16.4 and react-pointable at the same time and wanna have typings for both. I guess that's fine? People should not be allowed to do that.

[lostfictions]

@mDibyo oh right -- I forgot that DT types can have package.json files since this one doesn't. I guess it should probably declare it as a peerDependency rather than a dependency... but I also think declaring an upper range for React in the package.json wouldn't actually solve the CI problems either way. Can a maintainer confirm?

[typescript-bot]

@lostfictions Thank you for submitting this PR!

🔔 @johnnyreilly @bbenezech @pzavolinsky @digiguru @ericanderson @morcerf @tkrotoff @DovydasNavickas @onigoetz @theruther4d @guilhermehubner @ferdaber @jrakotoharisoa @iStefo @mDibyo - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead.

[typescript-bot]

A definition owner has approved this PR ⭐️. A maintainer will merge this PR shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for your contribution to DefinitelyTyped!

[ferdaber]

Changes look fine to me but if a DT maintainer can offer more input on how to best resolve the conflict with other libraries, that would be super awesome.

[sandersn]

This change looks fine. We don't have a good way to express version dependencies for this case, but I think people are likely to upgrade to React 16.4 at the same time that they stop using react-pointable, so it's OK to change it to be mostly useless. My reasoning is that people who currently use react-pointable presumably also use react, so they either pin the versions of both or neither. In other words, it would be weird for somebody to npm install @types/react-pointable followed by npm install @types/react@16.3.

You might try making react-pointable's onPointer* methods compatible with the new ones from react, but that's likely to be a lot of tricky work, and the resulting types will be (1) brittle (2) not correct in all cases.

Let me know if you want to me to go ahead and merge this. I think it's the best option given the crudity of versioning in Definitely Typed.

[lostfictions]

@sandersn If it seems like the best option to you that's fine by me!

I do still wonder if removing the react-pointable typings might make more sense -- I'm imagining a scenario where someone's stuck on an older version of React but discover they need pointer events, where if this PR is merged npm install @types/react-pointable gives them an unusable library. It'll be hard for someone to figure out why it's not working and that they need to specifically install a previous version of the types. (...Unless types-publisher can attach deprecation notices to packages?)

[sandersn]

@lostfictions I guess that React 17 would be the time to remove deprecated libraries from React 16.*. I don't know how strictly it follows semver. Regardless, I think that people will expect that if they want to npm install @types/react@16.3 then they will also need to install a specific version of react-pointable (and all the other libraries they want to use with 16.3). At least I hope that's the case.