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

mimic-fn dependency prevents use in some host environments #25

Closed
zenflow opened this issue Jan 29, 2019 · 3 comments
Closed

mimic-fn dependency prevents use in some host environments #25

zenflow opened this issue Jan 29, 2019 · 3 comments

Comments

@zenflow
Copy link
Contributor

zenflow commented Jan 29, 2019

After transpiling this dependency and all of it's dependencies (#23), this package still won't work on IE11 and other host environments (e.g. React Native on iOS) due to issue sindresorhus/mimic-function#10 which I would re-title "Error in host environments where Function instances have non-configurable properties".

My recommended solution is to remove the dependency on mimic-fn and make a new semver-major release, because:

  • what mimic-fn does is not needed to memoize functions, which seems to be the "one thing" this package is meant to do; the purpose of each is orthogonal
  • for users that need or desire the enhancement that mimic-fn provides, it is possible for them to apply mimic-fn to the function returned from mem, without breaking any functionality of either package. The only downside for them is that they need to apply it themselves, but that seems appropriate, given the first point.
@sindresorhus
Copy link
Owner

Sorry, I'm not interested in changing any code because of IE. I would recommend finding a different module :)

@zenflow
Copy link
Contributor Author

zenflow commented Jan 29, 2019

@sindresorhus Since it's not just IE11 that is affected, what about just wrapping the call to mimic-fn in a try block? That would not be a breaking change.

I.e. change this:

https://github.com/sindresorhus/mem/blob/bdfc93ad856fdf5622fc6406f601c9762370bc8a/index.js#L69

to this:

  try { 
    // the call to mimicFn below will throw in some host environments
    //   see https://github.com/sindresorhus/mimic-fn/issues/10
    mimicFn(memoized, fn); 
  } catch (error) {}

I would happily make the PR if you would accept that change

(Sorry to pester btw, and thanks for all your work)

@sindresorhus
Copy link
Owner

Alright. I'm ok with a try/catch.

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

No branches or pull requests

2 participants