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

TypeScript: constraint ref to element #20

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

OliverJAsh
Copy link

Previously this would not error at compile time:

import useComponentSize from '@rehooks/component-size';
import React = require('react');

declare const ref: React.RefObject<{ foo: 1 }>;
// No error
useComponentSize(ref);

With this change, it does.

useComponentSize always expects the ref to contain a HTML element, so it can reliably extract size/dimensions.

Copy link

@osdiab osdiab left a comment

Choose a reason for hiding this comment

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

This is faulty:

  1. I think Element is a strong enough constraint, allowing it to continue to work on SVG elements
  2. you can still keep it generic with T extends Element instead of removing the generic altogether

@OliverJAsh
Copy link
Author

  • I think Element is a strong enough constraint, allowing it to continue to work on SVG elements

Will fix

2. you can still keep it generic with T extends Element instead of removing the generic altogether

This function doesn't need to be generic, because the generic is only used for one parameter and it's not used in the return type.

Type parameters, that are only used once in the entire signature and don't have a constraint that contains another type parameter, are useless. They don't add type safety to the signature and can simply be replaced by their constraint or any if there is none.

https://github.com/fimbullinter/wotan/blob/def90abb18a3736d7b177a9f9a0b9b624b302f3a/packages/mimir/docs/no-misused-generics.md

https://twitter.com/SeaRyanC/status/1118634571564630016

@OliverJAsh OliverJAsh changed the title TypeScript: constraint ref to HTML element TypeScript: constraint ref to element Mar 4, 2020
@OliverJAsh
Copy link
Author

@osdiab Done

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.

2 participants