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

React Hooks #30036

Closed
Closed

Conversation

DanielRosenwasser
Copy link
Member

This PR introduces declarations for React's new hooks API. You can take a look at the new APIs here: https://reactjs.org/docs/hooks-reference.html

As a follow-up, working on this revealed problems for --strictFunctionTypes users, which #29889 fixes. That PR should be pulled in and ported to 16.7.

@typescript-bot typescript-bot added this to Needs Author Attention in Pull Request Status Board Oct 25, 2018
@typescript-bot typescript-bot added New Definition This PR creates a new definition package. Popular package This PR affects a popular package (as counted by NPM download counts). The Travis CI build failed labels Oct 25, 2018
@typescript-bot
Copy link
Contributor

typescript-bot commented Oct 25, 2018

@DanielRosenwasser Thank you for submitting this PR!

Because this is a new definition, a DefinitelyTyped maintainer will be reviewing this PR in the next few days once the Travis CI build passes.

In the meantime, if the build fails or a merge conflict occurs, I'll let you know. Have a nice day!

@typescript-bot
Copy link
Contributor

typescript-bot commented Oct 25, 2018

@DanielRosenwasser The Travis CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

@rhysforyou
Copy link
Contributor

rhysforyou commented Oct 26, 2018

While a breaking change, I think it also makes sense to rename SFC to FunctionComponent, as they're no longer necessarily stateless. The new docs even cover this, see this section on Hooks and Functional Components:

You might have previously known these as “stateless components”. We’re now introducing the ability to use React state from these, so we prefer the name “function components”.

Perhaps we can mitigate this by making SFC a deprecated alias for FunctionComponent?

@DanielRosenwasser
Copy link
Member Author

As an update, we can't create major.minor folders today in DefinitelyTyped. I'm currently thinking of just putting the features under 16.x and marking them as experimental.

I'm okay with renaming to FunctionComponent and creating a deprecated type alias, though it's not really worth the pain of removing it.

@Cryrivers
Copy link

@DanielRosenwasser just did the same thing and about to create a PR 😂. please also add FunctionComponent as well and deprecate SFC.

@Cryrivers
Copy link

is it possible to merge it now so we can play around?

// Hooks
// ----------------------------------------------------------------------

function useState<T>(initialState: T): [T, (newState: T) => void];

Choose a reason for hiding this comment

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

initialState can also be a function, I think?

Suggested change
function useState<T>(initialState: T): [T, (newState: T) => void];
function useState<T>(initialState: T | () => T): [T, (newState: T) => void];

Copy link
Contributor

@danielkcz danielkcz Oct 26, 2018

Choose a reason for hiding this comment

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

Also, the setState can be passed an updater function ... https://reactjs.org/docs/hooks-reference.html#functional-updates

Suggested change
function useState<T>(initialState: T): [T, (newState: T) => void];
function useState<T>(initialState: T | () => T): [T, (newState: T) => void | (updater: (prevState: T) => T)];

Please someone check it for me, I just wrote it blindly :)

Copy link
Member

Choose a reason for hiding this comment

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

(newState: T | ((prevState: T) => T)) => void

Copy link
Contributor

@danielkcz danielkcz Oct 26, 2018

Choose a reason for hiding this comment

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

Little side note, shouldn't the term nextState be used here? It's somewhat common convention I believe.

Copy link
Member

@Jessidhia Jessidhia left a comment

Choose a reason for hiding this comment

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

I happened to write #30057 while I was in a coding spree today 😅

I also included some jsdoc and a bunch of comments explaining why things are as they are so maybe we can join our efforts.

function useEffect(create: () => (void | (() => void)), inputs?: ReadonlyArray<unknown>): void;
function useContext<T>(foo: React.Context<T>): T;
function useReducer<S, A>(reducer: (state: S, action: A) => S, initialState: S): [S, (action: A) => void];
function useCallback<F extends (...args: never[]) => unknown>(callback: F, inputs?: ReadonlyArray<unknown>): F;
Copy link
Member

@Jessidhia Jessidhia Oct 26, 2018

Choose a reason for hiding this comment

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

I'd make the inputs argument required here and in useMemo because it makes no sense to use them without the memoization key; it'll just always regenerate the callback anyway.

I can maybe see a use for useMemo if you're giving it an external function that does an expensive computation, but if you give a function that can be its own memoization key to useCallback you didn't need useCallback to begin with.

Copy link
Contributor

@danielkcz danielkcz Oct 26, 2018

Choose a reason for hiding this comment

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

And are those inputs arguments required in the actual runtime code? It doesn't make much sense to enforce something in types that is not required by React itself. Although I agree with you it does not make sense without those :)

Copy link
Member

Choose a reason for hiding this comment

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

I don't think the types are there just to validate what "works" at runtime; they're also there to help you avoid logic errors if possible. Any use of useCallback(X) (without its second argument) is exactly equivalent to X but with more runtime and memory overhead.

Suggested change
function useCallback<F extends (...args: never[]) => unknown>(callback: F, inputs?: ReadonlyArray<unknown>): F;
// I made 'inputs' required here as there's no point to memoizing without the memoization key
/**
* `useCallback` will return a memoized version of the callback that only changes if one of the `inputs`
* has changed.
*
* @see https://reactjs.org/docs/hooks-reference.html#usecallback
*/
function useCallback<T extends (...args: never[]) => unknown>(callback: T, inputs: IdentityCheckInputList): T;

Copy link
Contributor

@danielkcz danielkcz Oct 31, 2018

Choose a reason for hiding this comment

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

@Kovensky My question still applies. Are those arguments required in React code? I haven't checked, but I am strongly against setting up types that are not enforced in runtime.

Besides, there is a useMemo which makes sense to have enforced inputs, but the useCallback does not need to rely on any inputs. It's just a function you want to stay the same over renders so child component does not rerender because of the change of it.

const onClick = useCallback(() => {
  // no arguments necessary here
})

return <NotAPure onClick={onClick} />


function useState<T>(initialState: T): [T, (newState: T) => void];
function useEffect(create: () => (void | (() => void)), inputs?: ReadonlyArray<unknown>): void;
function useContext<T>(foo: React.Context<T>): T;
Copy link
Member

Choose a reason for hiding this comment

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

This also accepts React.Consumer<T>, but I'm not 100% it is intentional or not.

I expect modules to be more forthcoming about exporting the Consumer than the Provider if they need to wrap the Provider with higher level logic...... or at least that has been the case with most context components I've written.

This also would break the const { Provider, Consumer } = pattern that's encouraged by React.

Copy link
Contributor

Choose a reason for hiding this comment

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

You are correct, it does accept Consumer directly. That's really awesome, I was a bit concerned about the need to export original "full context".

Edit react-hooks

Choose a reason for hiding this comment

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

Hello ✋ I'm also playing with this new hooks API and I found this inside the compile version of react@16.7.0-alpha.0 (node_modules/react/cjs/react.development.js:1454-1470)

function useContext(Context, observedBits) {
  var dispatcher = resolveDispatcher();
  {
    // TODO: add a more generic warning for invalid values.
    if (Context._context !== undefined) {
      var realContext = Context._context;
      // Don't deduplicate because this legitimately causes bugs
      // and nobody should be using this in existing code.
      if (realContext.Consumer === Context) {
        warning$1(false, 'Calling useContext(Context.Consumer) is not supported, may cause bugs, and will be ' + 'removed in a future major release. Did you mean to call useContext(Context) instead?');
      } else if (realContext.Provider === Context) {
        warning$1(false, 'Calling useContext(Context.Provider) is not supported. ' + 'Did you mean to call useContext(Context) instead?');
      }
    }
  }
  return dispatcher.useContext(Context, observedBits);
}

I don't really know why, but it's there! Just to help a bit with this PR that I'm really looking forward 😃

(Sorry, I would like to find this directly on the react source code on github, but I really don't know where it is 😅 )

Otherwise very nice job 👍 💯

Choose a reason for hiding this comment

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

image

The warning is also there in the @FredyC CodeSandbox console 😉

Copy link
Member

Choose a reason for hiding this comment

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

Interesting; I only knew of the warning for using Provider because it's also in the regular use of createContext. This warning is only in useContext and is the opposite of the warning in createContext about not using Context directly...

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice find @fabien0102, although not that nice in terms of a feature :) I've posted comment to RFC, but I doubt it will get attention there with a load of conversation happening there.

I guess that useContext is not meant to be used directly and we will have to export a custom hook for every reusable context access. Not cool, but it's probably better than exporting whole context object.

Choose a reason for hiding this comment

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

I don't really know why this useContext need the all context instead of the consumer, but I can tell that it works as expected with useContext(Context), I have a little POC here -> https://github.com/contiamo/restful-react/pull/79/files that is using this useContext and I have the correct and expected value from the <Provider /> 😉

Copy link

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
function useContext<T>(foo: React.Context<T>): T;
/**
* Accepts a context object (the value returned from `React.createContext`) and returns the current
* context value, as given by the nearest context provider for the given context.
*
* Note: while passing a Consumer would technically work, it would cause a runtime warning.
*
* @see https://reactjs.org/docs/hooks-reference.html#usecontext
*/
function useContext<T>(context: Context<T> /*, (not public API) observedBits?: number|boolean */): T;

// Hooks
// ----------------------------------------------------------------------

function useState<T>(initialState: T): [T, (newState: T) => void];
Copy link
Member

Choose a reason for hiding this comment

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

(newState: T | ((prevState: T) => T)) => void

function useReducer<S, A>(reducer: (state: S, action: A) => S, initialState: S): [S, (action: A) => void];
function useCallback<F extends (...args: never[]) => unknown>(callback: F, inputs?: ReadonlyArray<unknown>): F;
function useMemo<T>(create: () => T, inputs?: ReadonlyArray<unknown>): T;
function useRef<T extends unknown>(initialValue?: T): React.RefObject<T>;
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if initialValue is intended to be optional; similar to createContext, you're supposed to pass undefined if you want it to be undefined, even if it'll "work" without the argument. Even the code example in the docs specifically initializes their ref with null.

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems that we cannot use RefObject<T> here because its current property is readonly while useRef supports mutating the current

Copy link
Contributor

@ferdaber ferdaber Oct 30, 2018

Choose a reason for hiding this comment

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

We should actually be changing RefObject<T> to no longer have a readonly current property because it is actually mutable. <-- I made that type and I made the mistake 😅

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
function useRef<T extends unknown>(initialValue?: T): React.RefObject<T>;
/**
* `useRef` returns a mutable ref object whose `.current` property is initialized to the passed argument
* (`initialValue`). The returned object will persist for the full lifetime of the component.
*
* Note that `useRef()` is useful for more than the `ref` attribute. It’s handy for keeping any mutable
* value around similar to how you’d use instance fields in classes.
*
* @see https://reactjs.org/docs/hooks-reference.html#useref
*/
function useRef<T extends unknown>(initialValue: T): MutableRefObject<T>;

function useCallback<F extends (...args: never[]) => unknown>(callback: F, inputs?: ReadonlyArray<unknown>): F;
function useMemo<T>(create: () => T, inputs?: ReadonlyArray<unknown>): T;
function useRef<T extends unknown>(initialValue?: T): React.RefObject<T>;
function useImperativeMethods<T>(ref: React.Ref<T>, createInstance: () => T, inputs?: ReadonlyArray<unknown>): void;
Copy link
Member

@Jessidhia Jessidhia Oct 26, 2018

Choose a reason for hiding this comment

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

ref can be undefined (the React.forwardRef component can always be just not passed a ref; that just means it can forward the ref it is given one), but it definitely cannot be a string. I wrote a comment about this on your other PR.

Copy link
Member

@Jessidhia Jessidhia Oct 31, 2018

Choose a reason for hiding this comment

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

Suggested change
function useImperativeMethods<T>(ref: React.Ref<T>, createInstance: () => T, inputs?: ReadonlyArray<unknown>): void;
// NOTE: this does not accept strings, but that will have to be fixed by
// removing strings from type Ref<T> because of how forwardRef is defined.
/**
* `useImperativeMethods` customizes the instance value that is exposed to parent components when using
* `ref`. As always, imperative code using refs should be avoided in most cases.
*
* `useImperativeMethods` should be used with `React.forwardRef`.
*
* @see https://reactjs.org/docs/hooks-reference.html#useimperativemethods
*/
function useImperativeMethods<T>(ref: Ref<T>|undefined, init: () => T, inputs?: IdentityCheckInputList): void;

@gnapse
Copy link
Contributor

gnapse commented Oct 26, 2018

Anyway to use this even before merged? I'm crazy about playing around with hooks in a TS project I'm cooking, but the lack of type definitions is holding me back.

@targos
Copy link

targos commented Oct 26, 2018

@gnapse I got it to work by creating a react-hooks.d.ts file at the root of my project with the new functions:

import * as React from 'react';

declare module 'react' {
  //
  // Hooks
  // ----------------------------------------------------------------------
  function useState<T>(initialState: T | (() => T)): [T, (newState: T) => void];
  function useEffect(
    create: () => void | (() => void),
    inputs?: ReadonlyArray<unknown>
  ): void;
  function useContext<T>(foo: React.Context<T>): T;
  function useReducer<S, A>(
    reducer: (state: S, action: A) => S,
    initialState: S
  ): [S, (action: A) => void];
  function useCallback<F extends (...args: never[]) => unknown>(
    callback: F,
    inputs?: ReadonlyArray<unknown>
  ): F;
  function useMemo<T>(create: () => T, inputs?: ReadonlyArray<unknown>): T;
  function useRef<T extends unknown>(initialValue?: T): React.RefObject<T>;
  function useImperativeMethods<T>(
    ref: React.Ref<T>,
    createInstance: () => T,
    inputs?: ReadonlyArray<unknown>
  ): void;
  const useMutationEffect: typeof useEffect;
  const useLayoutEffect: typeof useEffect;
}

// ----------------------------------------------------------------------

function useState<T>(initialState: T): [T, (newState: T) => void];
function useEffect(create: () => (void | (() => void)), inputs?: ReadonlyArray<unknown>): void;
Copy link
Contributor

@danielkcz danielkcz Oct 27, 2018

Choose a reason for hiding this comment

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

The useEffect should not be expecting to return void but unknown instead. This is most important for async/await which returns Promise.

Suggested change
function useEffect(create: () => (void | (() => void)), inputs?: ReadonlyArray<unknown>): void;
function useEffect(create: () => (unknown | (() => unknown)), inputs?: ReadonlyArray<unknown>): void;

@tgriesser
Copy link
Contributor

FYI: this PR makes use of unknown quite a bit, but unknown is only available in TS 3+

@danielkcz
Copy link
Contributor

@tgriesser That might be right, but imo if someone is willing to use such bleeding edge technology as hooks then they most likely don't have a problem with getting the latest TypeScript as well. Thus I don't agree we should be held hostage by this theoretical group of people who would choose such a path.

@tgriesser
Copy link
Contributor

@FredyC mostly pointing it out because the Definitions here say 2.8.

@Jessidhia
Copy link
Member

Jessidhia commented Oct 30, 2018 via email

@jtmthf
Copy link
Contributor

jtmthf commented Oct 30, 2018

@DanielRosenwasser would you be okay with adding a new type alias named FC defined as type FC<P = {}> = FunctionComponent<P>? It would be nice to keep a short type alias that is not deprecated.

@rhysforyou
Copy link
Contributor

I assumed the original choice to use SFC was based on how long and unwieldy StatelessFunctionComponent was to type, and I don't think that's as much of a problem with FunctionComponent.

@danielkcz
Copy link
Contributor

I would be kinda inclined to introduce a new naming like HookedComponent, which is still long, but not too bad. Adding an alias for HC could be confusing in comparison to HOC, so that's probably bad idea.

@Jessidhia
Copy link
Member

We can't know at a type level if the component has hooks or not; it's just a function, and to typescript a hooked component is identical to a stateless one. This is strictly an issue of naming.

function useEffect(create: () => (void | (() => void)), inputs?: ReadonlyArray<unknown>): void;
function useContext<T>(foo: React.Context<T>): T;
function useReducer<S, A>(reducer: (state: S, action: A) => S, initialState: S): [S, (action: A) => void];
function useCallback<F extends (...args: never[]) => unknown>(callback: F, inputs?: ReadonlyArray<unknown>): F;
Copy link
Contributor

Choose a reason for hiding this comment

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

Even though the useCallback its self doesn't receive the inputs as args, it can be still be called with args, e.g. as an event handler.

Suggested change
function useCallback<F extends (...args: never[]) => unknown>(callback: F, inputs?: ReadonlyArray<unknown>): F;
function useCallback<F extends (...args: any[]) => unknown>(callback: F, inputs?: ReadonlyArray<unknown>): F;

Copy link
Member Author

@DanielRosenwasser DanielRosenwasser Oct 31, 2018

Choose a reason for hiding this comment

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

Why? This constraint doesn't imply that the function you pass can't be called outside. (...args: never[]) => unknown is effectively "any function".

Copy link
Member

Choose a reason for hiding this comment

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

It's a consequence of some newer assignability rules since 3.0(?) which makes never in this position even more permissible than any.

I don't understand any of it either I just know that that's how it's working 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah got it, maybe the issue I saw was another consequence of being on 2.9.1

Copy link
Member

@Jessidhia Jessidhia left a comment

Choose a reason for hiding this comment

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

I made suggested changes with (improved versions of) the types I have written in my own PR (#30057); while they are for the most part identical to your current definitions, they also have niceties such as JSDoc comments, and fix a few bugs.

//
// Hooks
// ----------------------------------------------------------------------

Copy link
Member

@Jessidhia Jessidhia Oct 31, 2018

Choose a reason for hiding this comment

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

These are some type definitions that I make use of in my version of the type definitions. The names or the values are mainly based on React's own Flow type definitions.

Suggested change
// Unlike the class component setState, the updates are not allowed to be partial
type SetStateAction<S> = S | ((prevState: S) => S);
// this technically does accept a second argument, but it's already under a deprecation warning
// and it's not even released so probably better to not define it.
type Dispatch<A> = (value: A) => void;
// Unlike redux, the actions _can_ be anything
type Reducer<S, A> = (prevState: S, action: A) => S;
// The identity check is done with the SameValue algorithm (Object.is), which is stricter than ===
type IdentityCheckInputList = ReadonlyArray<unknown>;
// NOTE: the effect callbacks are actually allowed to return anything, but functions are treated
// specially. I don't think it's intended to accept async functions, should use throw instead of
// await to properly engage Suspense.
type EffectCallback = () => (void | (() => void));
interface MutableRefObject<T> {
current: T;
}

// Hooks
// ----------------------------------------------------------------------

function useState<T>(initialState: T): [T, (newState: T) => void];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
function useState<T>(initialState: T): [T, (newState: T) => void];
/**
* Returns a stateful value, and a function to update it.
*
* @see https://reactjs.org/docs/hooks-reference.html#usestate
*/
function useState<S>(initialState: S | (() => S)): [S, Dispatch<SetStateAction<S>>];

Copy link
Contributor

Choose a reason for hiding this comment

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

Use correct terminology perhaps?

     /**
-     * Returns a stateful value, and a function to update it.
+     * Returns a tuple with stateful value, and a function to update it.
      *
      * @see https://reactjs.org/docs/hooks-reference.html#usestate
      */
     function useState<S>(initialState: S | (() => S)): [S, Dispatch<SetStateAction<S>>];

// ----------------------------------------------------------------------

function useState<T>(initialState: T): [T, (newState: T) => void];
function useEffect(create: () => (void | (() => void)), inputs?: ReadonlyArray<unknown>): void;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
function useEffect(create: () => (void | (() => void)), inputs?: ReadonlyArray<unknown>): void;
/**
* Accepts a function that contains imperative, possibly effectful code.
*
* @param effect Imperative function that can return a cleanup function
* @param inputs If present, effect will only activate if the values in the list change.
*
* @see https://reactjs.org/docs/hooks-reference.html#useeffect
*/
function useEffect(effect: EffectCallback, inputs?: IdentityCheckInputList): void;


function useState<T>(initialState: T): [T, (newState: T) => void];
function useEffect(create: () => (void | (() => void)), inputs?: ReadonlyArray<unknown>): void;
function useContext<T>(foo: React.Context<T>): T;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
function useContext<T>(foo: React.Context<T>): T;
/**
* Accepts a context object (the value returned from `React.createContext`) and returns the current
* context value, as given by the nearest context provider for the given context.
*
* Note: while passing a Consumer would technically work, it would cause a runtime warning.
*
* @see https://reactjs.org/docs/hooks-reference.html#usecontext
*/
function useContext<T>(context: Context<T> /*, (not public API) observedBits?: number|boolean */): T;

function useState<T>(initialState: T): [T, (newState: T) => void];
function useEffect(create: () => (void | (() => void)), inputs?: ReadonlyArray<unknown>): void;
function useContext<T>(foo: React.Context<T>): T;
function useReducer<S, A>(reducer: (state: S, action: A) => S, initialState: S): [S, (action: A) => void];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
function useReducer<S, A>(reducer: (state: S, action: A) => S, initialState: S): [S, (action: A) => void];
/**
* An alternative to `useState`.
*
* `useReducer` is usually preferable to `useState` when you have complex state logic that involves
* multiple sub-values. It also lets you optimize performance for components that trigger deep
* updates because you can pass `dispatch` down instead of callbacks.
*
* @see https://reactjs.org/docs/hooks-reference.html#usereducer
*/
function useReducer<S, A>(reducer: Reducer<S, A>, initialState: S, initialAction?: A | null): [S, Dispatch<A>];

Copy link
Member

@Jessidhia Jessidhia Oct 31, 2018

Choose a reason for hiding this comment

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

useState itself could be defined in terms of useReducer if you try really hard. That's how it works internally anyway, and the Flow types come pretty close to representing it.

type SetStateReducer<S> = Reducer<S, SetStateAction<S>>

function useState<S>(initialState: S): ReturnType<
  typeof useReducer<SetStateReducer<S>, SetStateAction<S>>
>

function useContext<T>(foo: React.Context<T>): T;
function useReducer<S, A>(reducer: (state: S, action: A) => S, initialState: S): [S, (action: A) => void];
function useCallback<F extends (...args: never[]) => unknown>(callback: F, inputs?: ReadonlyArray<unknown>): F;
function useMemo<T>(create: () => T, inputs?: ReadonlyArray<unknown>): T;
Copy link
Member

@Jessidhia Jessidhia Oct 31, 2018

Choose a reason for hiding this comment

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

Note that, unless useMemo is given a referentially stable function (e.g. a function declared outside the component, or imported from somewhere), it will be useless without also receiving the inputs array for the same reason as useCallback. I thought of making it required here too but there actually is a valid use of it without inputs so I kept it optional.

Suggested change
function useMemo<T>(create: () => T, inputs?: ReadonlyArray<unknown>): T;
/**
* `useMemo` will only recompute the memoized value when one of the `inputs` has changed.
*
* @see https://reactjs.org/docs/hooks-reference.html#usememo
*/
function useMemo<T>(factory: () => T, inputs?: IdentityCheckInputList): T;

function useReducer<S, A>(reducer: (state: S, action: A) => S, initialState: S): [S, (action: A) => void];
function useCallback<F extends (...args: never[]) => unknown>(callback: F, inputs?: ReadonlyArray<unknown>): F;
function useMemo<T>(create: () => T, inputs?: ReadonlyArray<unknown>): T;
function useRef<T extends unknown>(initialValue?: T): React.RefObject<T>;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
function useRef<T extends unknown>(initialValue?: T): React.RefObject<T>;
/**
* `useRef` returns a mutable ref object whose `.current` property is initialized to the passed argument
* (`initialValue`). The returned object will persist for the full lifetime of the component.
*
* Note that `useRef()` is useful for more than the `ref` attribute. It’s handy for keeping any mutable
* value around similar to how you’d use instance fields in classes.
*
* @see https://reactjs.org/docs/hooks-reference.html#useref
*/
function useRef<T extends unknown>(initialValue: T): MutableRefObject<T>;

function useCallback<F extends (...args: never[]) => unknown>(callback: F, inputs?: ReadonlyArray<unknown>): F;
function useMemo<T>(create: () => T, inputs?: ReadonlyArray<unknown>): T;
function useRef<T extends unknown>(initialValue?: T): React.RefObject<T>;
function useImperativeMethods<T>(ref: React.Ref<T>, createInstance: () => T, inputs?: ReadonlyArray<unknown>): void;
Copy link
Member

@Jessidhia Jessidhia Oct 31, 2018

Choose a reason for hiding this comment

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

Suggested change
function useImperativeMethods<T>(ref: React.Ref<T>, createInstance: () => T, inputs?: ReadonlyArray<unknown>): void;
// NOTE: this does not accept strings, but that will have to be fixed by
// removing strings from type Ref<T> because of how forwardRef is defined.
/**
* `useImperativeMethods` customizes the instance value that is exposed to parent components when using
* `ref`. As always, imperative code using refs should be avoided in most cases.
*
* `useImperativeMethods` should be used with `React.forwardRef`.
*
* @see https://reactjs.org/docs/hooks-reference.html#useimperativemethods
*/
function useImperativeMethods<T>(ref: Ref<T>|undefined, init: () => T, inputs?: IdentityCheckInputList): void;

function useMemo<T>(create: () => T, inputs?: ReadonlyArray<unknown>): T;
function useRef<T extends unknown>(initialValue?: T): React.RefObject<T>;
function useImperativeMethods<T>(ref: React.Ref<T>, createInstance: () => T, inputs?: ReadonlyArray<unknown>): void;
const useMutationEffect: typeof useEffect;
Copy link
Member

Choose a reason for hiding this comment

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

(this is the same type, it's just done as a full function declaration so jsdoc can be attached to it)

Suggested change
const useMutationEffect: typeof useEffect;
/**
* The signature is identical to `useEffect`, but it fires synchronously during the same phase that
* React performs its DOM mutations, before sibling components have been updated. Use this to perform
* custom DOM mutations.
*
* Prefer the standard `useEffect` when possible to avoid blocking visual updates.
*
* @see https://reactjs.org/docs/hooks-reference.html#usemutationeffect
*/
function useMutationEffect(effect: EffectCallback, inputs?: IdentityCheckInputList): void;

function useRef<T extends unknown>(initialValue?: T): React.RefObject<T>;
function useImperativeMethods<T>(ref: React.Ref<T>, createInstance: () => T, inputs?: ReadonlyArray<unknown>): void;
const useMutationEffect: typeof useEffect;
const useLayoutEffect: typeof useEffect;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const useLayoutEffect: typeof useEffect;
/**
* The signature is identical to `useEffect`, but it fires synchronously after all DOM mutations.
* Use this to read layout from the DOM and synchronously re-render. Updates scheduled inside
* `useLayoutEffect` will be flushed synchronously, before the browser has a chance to paint.
*
* Prefer the standard `useEffect` when possible to avoid blocking visual updates.
*
* If you’re migrating code from a class component, `useLayoutEffect` fires in the same phase as
* `componentDidMount` and `componentDidUpdate`.
* @see https://reactjs.org/docs/hooks-reference.html#uselayouteffect
*/
function useLayoutEffect(effect: EffectCallback, inputs?: IdentityCheckInputList): void;

Copy link
Member

Choose a reason for hiding this comment

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

JSDoc is super useful from a users point of view; intellisense is an awesome feature! If there's no objection I wonder if it might be worth taking @Kovensky's PR in place of this?

Copy link
Contributor

Choose a reason for hiding this comment

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

There is one obvious difference that @Kovensky's types goes directly into the "release" version while these ones are completely separated. However, considering that these are purely additive I don't think there is any harm to have a proposal types in official package.

@brunolemos
Copy link
Contributor

brunolemos commented Oct 31, 2018

Dan just reinforced the suggestion to call it Function Components instead of Stateless Function Components (SFC).

We recently renamed all XxxProperties to XxxProps on react-native without making a breaking change, we can do it here too. Keep the old types, mark them as deprecated by adding a /** @deprecated Use XXX */ comment on top of each one and alias them to the new type. Here's the commit.

@typescript-bot
Copy link
Contributor

@DanielRosenwasser I haven't seen anything from you in a while and this PR currently has problems that prevent it from being merged. The PR will be closed tomorrow if there aren't new commits to fix the issues.

@DanielRosenwasser
Copy link
Member Author

Closing in favor of #30057. Thanks @Kovensky!

@typescript-bot typescript-bot removed this from Needs Author Attention in Pull Request Status Board Nov 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Definition This PR creates a new definition package. Popular package This PR affects a popular package (as counted by NPM download counts).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet