immerjs / use-immer

Use immer to drive state with a React hooks
MIT License
4.04k stars 92 forks source link

Doesn't work in IE11 #13

Closed stnwk closed 5 years ago

stnwk commented 5 years ago

Hi :)

Today I was bitten by IE11 when using this little library, because it broke our build (since we don't transpile our node_modules). We learned that the following code line:

https://github.com/mweststrate/use-immer/blob/d7bd530627a368dc74dde5ae9e3fccb9c4c4ea6c/index.js#L5

was responsible for it. More precisely the lack of support for destructuring in IE11 + the omitted build step resulted in a syntax error which crashed our app.

I do realise why you decided to omit a build step for this tiny function, but following your comment and reasoning in https://github.com/mobxjs/mobx-react-lite/issues/63#issuecomment-462675248: I was caught off-guard and definitely did not expect this library to break our app in IE11, especially since immer is also working without a problem.

I hope you see a reason to transpile even this tiny function to valid ES5-compatible code and maybe you'll find some time to add the build step.

Alternatively I could prepare a PR that gets rid of the destructuring similar to this:

module.exports.useImmer = function useImmer(initialValue) {
  const state = React.useState(initialValue);

  return [
    state[0],
    updater => {
      state[1](produce(updater));
    }
  ];
}

Since we needed a fix asap, our solution was to just copypasta the code in our codebase. We use typescript, so I tried to combine your code with the provided typings from index.d.ts:

import React from 'react';
import produce, { Draft } from 'immer';

type Recipe<S = any> = (this: Draft<S>, draftState: Draft<S>) => void | S;
type Update<S = any> = (recipe: Recipe<S>) => void;

export const useImmer = <S = any>(initialValue: S): [S, Update<S>] => {
  const [value, updateValue] = React.useState(initialValue);

  return [
    value,
    (updater) => {
      updateValue((state) => produce(state, updater));
      // ^ type error much
    },
  ];
};

See https://codesandbox.io/s/5ymlw2wjrx - but that didn't work well, since typescript was complaining about a type mismatch.

Our solution was to change the Recipe typing like this:

// does not work
type Recipe<S = any> = (this: Draft<S>, draftState: Draft<S>) => void | S;

// works
type Recipe<S = any> = (this: Draft<S>, draftState: Draft<S>) => void;

So we just omitted | S from the returned type and that seemed to fix it, but I am unsure why it was there in the first place. Maybe you can elaborate on that?

Anyways, thanks for your time & work. Looking forward to an answer :)

mweststrate commented 5 years ago

Added a build step, so should be fixed in 0.2.1

The | S is there to be able to replace the state by returning a new object from the recipe

stnwk commented 5 years ago

Thanks!

SergeyVolynkin commented 5 years ago

@mweststrate Does immer work in IE 11?

I thought that immer uses Proxy object internally so it can't be applied in the projects aiming to support IE 11 (https://caniuse.com/#feat=proxy) and there is no polyfill for that functionality…

mweststrate commented 5 years ago

Immer supports IE 11. For details see readme

Op do 30 mei 2019 12:59 schreef Sergey Volynkin notifications@github.com:

@mweststrate https://github.com/mweststrate Does immer work in IE 11?

I thought that immer uses Proxy object internally so it can't be applied in the projects aiming to support IE 11. (https://caniuse.com/#feat=proxy)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/immerjs/use-immer/issues/13?email_source=notifications&email_token=AAN4NBBTS7BDRVDGQHLQGXTPX6XQ3A5CNFSM4HDVZXIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWSBEIA#issuecomment-497291808, or mute the thread https://github.com/notifications/unsubscribe-auth/AAN4NBD2V5KQETUMDPFNJT3PX6XQ3ANCNFSM4HDVZXIA .