Promise.finally is being overwritten in React Native

[benjaminreid]

Describe the bug
The inclusion of Storybook inside a React Native app seems to remove the implementation of Promise.prototype.finally. Causing the Promise.resolve(...).finally is not a function error to occur.

To Reproduce
Steps to reproduce the behavior:

1. Create a fresh React Native project.
2. Setup Storybook as reference here, https://storybook.js.org/docs/guides/guide-react-native/ npx -p @storybook/cli sb init --type react_native (no server)
3. Add the code snippet reference below and you’ll see initially you can call toString on Promise.prototype.finally but after importing Storybook (you have to use require in this instance, not import, so the ordering works for the test to see that finally starts off being an actual function) it’s no undefined.

Example repo here, https://github.com/benjaminreid/SBPromiseTest

Expected behavior
Promise.prototype.finally to no be removed.

Screenshots
N/A

Code snippets
finally is valid here, but after the inclusion of Storybook, it’s undefined.

console.log('Before:', Promise.prototype.finally.toString());

const StoryBook = require('./storybook');

console.log('After:', Promise.prototype.finally);

System:

Environment Info:

  System:
    OS: macOS Mojave 10.14.6
    CPU: (4) x64 Intel(R) Core(TM) i5-7360U CPU @ 2.30GHz
  Binaries:
    Node: 10.16.0 - /var/folders/94/btf_scw12xbdnqv5dtv2t2hr0000gn/T/fnm-shell-8148567/bin/node
    Yarn: 1.17.3 - /usr/local/bin/yarn
    npm: 6.9.0 - /var/folders/94/btf_scw12xbdnqv5dtv2t2hr0000gn/T/fnm-shell-8148567/bin/npm
  Browsers:
    Chrome: 77.0.3865.90
    Firefox: 69.0.1
    Safari: 13.0.2
  npmPackages:
    @storybook/react-native: ^5.3.0-alpha.17 => 5.3.0-alpha.17

Additional context
In my instance redux-persist use this in their onBeforeLift callback.

[tjbenton]

in case anyone didn't figure out this simple temp fix.

// temp fix for the promise.finally
// https://github.com/storybookjs/storybook/issues/8371
const fn = Promise.prototype.finally

const StoryBook = require('./storybook')

Promise.prototype.finally = fn

[odjhey]

Hello, thank you for this
Quick fix is working on OSX but not on windows, any idea why?

in case anyone didn't figure out this simple temp fix.

// temp fix for the promise.finally
// https://github.com/storybookjs/storybook/issues/8371
const fn = Promise.prototype.finally

const StoryBook = require('./storybook')

Promise.prototype.finally = fn

[tjbenton]

@odjhey The only reason why I can think of for why this wouldn't work is that the version of node you're using doesn't support it. But if the node --version is the same on both Windows and OSX then i'm not sure. You could also try to just import the pollyfill for it if upgrading your node version doesn't work.

[limaAniceto]

Hey there @tjbenton, what if you are running storybook as a View contained in an app?

Doing the same as you did, before/after the imports doesn't change it.

Minor update: If I do your tmp fix on @storybook/react-native/index.js it works. However then I'd have to fork the Storybook repo just for this (or patch-package it)

[Gongreg]

Hey,
Currently we cannot give any good solution for this problem. To overcome this issue we would have to change compilation target, but most of storybook packages are targeting web. So fixing it would take a while. But we will try to think of something. :)

[benjaminreid]

@Gongreg Thanks for looking into this, I appreciate it’s a tricky problem. Hopefully someone can find a solution soon.

[Gongreg]

The longer term solution is we are going to try splitting out react native storybook out of monorepo, so we will be able to change build setup. But that will take time.

If anyone has any short term ideas it would be great, because I am also experiencing the issue in our own project. :/

[shilman]

Gadzooks!! I just released https://github.com/storybookjs/storybook/releases/tag/v5.3.0-alpha.42 containing PR storybookjs/storybook#8698 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.

Closing this issue. Please re-open if you think there's still more to do.

[Gongreg]

@shilman this issue is a lot bigger actually :| All of the packages that are used in React Native shouldnt have any polyfills. We cant think of an easy solution with @ndelangen . Maybe we could just add the polyfills in entry files for manager/preview and skip them in lib/addons?

[joaocarmo]

A good set it and forget it workaround for this issue is a combination of the solutions presented by @tjbenton and @limaAniceto above with patch-package:

1. Open the entry file

vi node_modules/@storybook/react-native/dist/index.js 

2. Append the fixes to the start and end of the file

var fnFix = Promise.prototype.finally;
/*original code*/
Promise.prototype.finally = fnFix;

3. Save and commit the patch to your repository

yarn patch-package @storybook/react-native && git add patches

This will ensure you do not include in your project patches to modules, that the package is always patched for every developer in your team and can even be included in CI/CD pipelines and containers.

The diff is saved in a patches directory and when the faulty module gets fixed, you can just remove it.

[colinta]

This issue just cost me a night and a morning trying to hunt down what was going on, so to find out it's a storybook issue and it's been known about really has steam coming out of my ears.

[benjaminreid]

@colinta You’ve just reminded me this issue exists, I was hoping with version 6 this may have been closed. Shame if it’s still hanging around.

[dannyhw]

looking into a fix for this, seems like just changing the babel config could resolve this.

__

edit: maybe not as simple as expected still investigating

[dannyhw]

This is from corejs and it seems like the problem starts with storybook core because even if I remove all the old polyfills from the @storybook/react-native config then it still happens.

The babel preset env polyfill will cause this when ever 'core-js/modules/es.promise' gets imported, see the following issues:

zloirock/core-js#749
babel/babel#11742

[apedroferreira]

@dannyhw any updates? has this been fixed?

[dannyhw]

@apedroferreira unfortunately not since the issue is upstream I wasn't able to make progress on this. I'm hoping that maybe upgrading dependencies to 6.0+ will help. I'll test it in the 6.0 upgrade branch.

From what I understand there would need to be a version of storybook client that isn't polyfilled or at least with a different browser target. So I think a workaround is the best chance we have for now, however I will raise the issue on the storybook repo to see if we can look into it.

Edit: In the past I did also try applying the workaround above to the project and it doesn't seem to work for me

[ajacquierbret]

@dannyhw Today, after doing an upgrade of the Expo SDK (40), the same error appeared.

After some investigations, more or less dumb tries of making my app working, I discovered something quite interesting.

If I add import 'core-js/features/promise/finally' to @storybook/react-native/dist/index.js it still shows an error.
But if I remove the import and add it again after my app is fully loaded, and then save the file, the Fast Refresh feature of react-native handles the import and then everything works until the next JS bundle reload.

That means that if I want to work on my app, I need to add this import after every bundle reload. And if I want to ship it in production, I need to do the same procedure before doing any build. I desperately need help.

I honestly don't know what to do with this info but maybe someone will.

Btw, the above workaround did work for a while, but only for a while. I'm now quite stuck with this error and don't have any clue on how to get my app working. Any fix or workaround would be HUGELY appreciated.

[ajacquierbret]

Ok I managed to get my app working !

Adding an import of core-js/features/promise AFTER importing Storybook is the fix. Above workarounds and patches don't work because Promise.prototype.finally keeps being overwritten even after the execution of @storybook/react-native/dist/index.js, maybe because of some Storybook plugins.

In my case, I had to import core-js/features/promise a bit deep in my app hierarchy, but this effectively resolves the issue.

Anyway, some consideration about this issue from the Storybook team would be highly appreciated, since any RN developer working with promise based libs (e.g. @apollo/client 3.x) will be facing the same issue...

[dannyhw]

@ajacquierbret hey thanks for posting your workaround hopefully that will save some people a lot of frustration.

I've tried different things in the past to resolve this and the proper fix always seems to require changes upstream. It's hard to coordinate these changes since it's only rn with this problem. I think in the web versions it's fixed via webpack wizardry.

I was kind of hoping that 6.0 might fix this but I'm not sure yet. I'll try it out in the upgrade branch soon.

[4lun]

Posting in case I can help save someone a few hours of their time, it looks like this polyfill behaviour of Storybook breaks promise.resolve in Native Module contexts too.

Example in a native Android module

@ReactMethod
fun multiply(a: Int, b: Int, promise: Promise) {
  promise.resolve(a * b)
}

Importing any part of Storybook in a project breaks the above example (the promise is never resolved on the RN/JS side), e.g.

import {getStorybookUI} from '@storybook/react-native';
getStorybookUI({});

// The following will now never resolve (but did work prior to the inclusion of Storybook)
AwesomeModule.multiply(3,7).then(...); // or
(async () => { const res = await AwesomeModule.multiply(3,7); ... })();

MVP repo with screenshots, basic example bootstrapped using react-native init & create-react-native-library

Have tried the potential fixes mentioned in this thread, but currently I've been unsuccessful in figuring a way around the issue.

[mmoio]

Following the approach of @defrex comment i've been able to get it to work with react lazy and suspense:

StorybookScreen.tsx

const StorybookUI = lazy(() => import('../../storybook'));

const StorybookScreen: React.FC = () => {
  return (
    <Suspense fallback={<Text> loading </Text>}>
      <StorybookUI />
    </Suspense>
  );
};

App.tsx

const App: React.FC = () => (
  { __DEV__? <StorybookScreen/> : <App/> }
);

[dannyhw]

In this PR I demonstrate how you can use patch package to fix this error.

Basically just this patch for @storybook/addons

diff --git a/node_modules/@storybook/addons/dist/index.js b/node_modules/@storybook/addons/dist/index.js
index 589d611..81bc9e7 100644
--- a/node_modules/@storybook/addons/dist/index.js
+++ b/node_modules/@storybook/addons/dist/index.js
@@ -8,8 +8,6 @@ require("core-js/modules/es.object.to-string");
 
 require("core-js/modules/es.object.values");
 
-require("core-js/modules/es.promise");
-
 require("core-js/modules/web.dom-collections.for-each");
 
 Object.defineProperty(exports, "__esModule", {
 

[pke]

None of the proposed fixes work. Not even after restarting the metro server.

Removing the import from @storybook/addons/dist/index.js causes a stack exceed exception on app start.

I also tried not to call getStorybookUI and that causes some weird timeout problems in core-js microtasks.

At this state the lib is not usable if it destroys the RN promise implementation.

The only way to get this to work is to use a timeout (of 10 ms) before loading any storybook code.

function StorybookScreen() {
  const [storybookUI, setStorybookUI] = React.useState<ReactNode | null>(null)

  useEffect(() => {
    if (!__DEV__) {
      return
    }
    // eslint-disable-next-line @typescript-eslint/no-var-requires
    const timer = setTimeout(() => {
      const { StorybookUI } = require("../storybook")
      setStorybookUI(<StorybookUI />)
    }, 10)
    return () => clearTimeout(timer)
  }, [setStorybookUI])

  return (
    <>
      {storybookUI}
     </>
  )
}
`` 

[dannyhw]

@pke patching the addons dependency to remove the corejs promise polyfill works as a workaround as far as I know.

Also there was a new version yesterday to fix some issues in the latest version of react native, not sure if its related to any issues you're having.

[pke]

Removing the polyfill didn't work in my setup. I tried for hours. It worked in hot reload sometimes but not deterministic. Cold boot the app never worked. The timeout works. It's clearly a race condition and my setup might be slower or faster than the rest of the ones who tries the workarounds.

I am on a fairly old RN version 66.3 and I still don't understand how the polyfill can even be active when RN provides a complete es6 promise including finally. I'd like to understand how the polyfill works.

In the corejs I found related bug and they clearly stated the way storybook authors use the polyfills is wrong. They shouldn't be imported from the modules folder.

[dannyhw]

@pke in the v6 beta we get around this problem by resolving to the modern build of storybook (bundled as sbmodern)

[pke]

now just checking back with my app its not working again... haha seems to be clock related or something weird.
Anyway... I am going to give v6 a try. However I really would like to use the webserver as a co-pilot, which is not yet ready in v6 iiuc?

[dannyhw]

The server is not ready and probably won't be for some time. I don't really know how to make it work for v6 and if I were to recreate it I'd probably opt for implementing something a bit different.

As always if someone is willing to figure it out then I'll happily take a PR but I've tried a few times and it seems to not be compatible without making a lot of changes.

[pke]

@pke patching the addons dependency to remove the corejs promise polyfill works as a workaround as far as I know.

I tried again removing the import and now it corejs runs into a stack overflow cause no promise seems to be defined and it uses some alternative setTimeout path or so.

That is on RN 66.3

[dannyhw]

@pke if you have a reproduction I can try to debug it for you.

[pke]

Thanks @dannyhw, I can't produce a version where its easily reproducable, thats the problem. It seems to be totally random.
What I want to understand is: how does the polyfill thinks it needs to engage? RN has a complete Promise implementation and it should be loaded with RN at boot and therefore available when core-js runs.
Or has it something to do with RN itself depending on core-js internally, but just a different version than storybook addons?

[Waltari10]

Hey, I tried upgrading to Storybook 6 and I still experience this problem with Apollo-client. Removing storybook fixes the issue. Any idea what to do?

"@storybook/addon-actions": "6.5.13",
"@storybook/react-native": "6.0.1-beta.10",
"react-native": "0.70.3",
"@apollo/client": "3.7.1",

[dannyhw]

@Waltari10 make sure you add the metro config to resolve sbmodern.

[MiguelAraCo]

An alternative solution that should work for any version is to configure Metro so that it doesn't load the polyfill that Storybook packages load.

Inside metro.config.js add something like this:

const { getDefaultConfig } = require('expo/metro-config');

const config = getDefaultConfig(__dirname);

config.resolver.resolveRequest = (context, moduleName, platform) => {
  if (moduleName.startsWith('core-js/modules/es.promise')) {
    // Storybook patches Promise and removes finally, causing errors throughout the application
    // This configures it so that the offending package isn't loaded when requested
    return {
      type: 'empty',
    };
  } else {
    return context.resolveRequest(context, moduleName, platform);
  }
};

module.exports = config;

[tj-mc]

Is there a more up-to-date solution to this? I'm trying to use @rnx-kit/metro-resolver-symlinks, but I must overwrite resolveRequest. It seems like this issue is affecting almost everyone using storybook in React Native, and it can be very difficult to find out that storybook is the source of it.

[tj-mc]

I have updated to the latest version, as I saw in a changelog that this issue was fixed, however it persists in my case
I have set

config.resolver.resolverMainFields = ['sbmodern', 'react-native', 'browser', 'main'];

In metro.config.js

[dannyhw]

@tj-mc what version are you using? Its only fixed in 6.5.

[tj-mc]

Turns out I needed to upgrade the other @storybook deps too. For anyone solving this issue, here is what I needed.

 "@storybook/addon-actions": "^7.0.9",
 "@storybook/addon-knobs": "^7.0.2",
 "@storybook/addon-links": "^7.0.9",
 "@storybook/addon-ondevice-actions": "^6.5.3",
 "@storybook/addon-ondevice-knobs": "^6.5.3",
 "@storybook/react-native": "^6.5.3",
 "@storybook/react-native-server": "^6.5.3",
 
  "react-native": "0.69.8",

In metro.config.js:

config.resolver.resolverMainFields = ['sbmodern', 'react-native', 'browser', 'main'];

The last 3 values are the default for React Native.

And for the record, this is an ejected Expo app. Now working fine without any errors. Thank you for your work on this! ❤️

If you were previously using the above custom-resolver fix, you can now delete it 😄