Promise.finally is being overwritten in React Native about react-native HOT 46 CLOSED

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.

from react-native.

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

from react-native.

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...

from react-native.

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;

from react-native.

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)

from react-native.

@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?

from react-native.

Since as yet there is no @storybook/react-native@6, I've found a more elegant workaround.

The below example lazy-loads Storybook. Convenient both to avoid polyfill downgrades as well as tree-shaking the code out of production builds.

import React, { ReactNode, useEffect, useState } from 'react'

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

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

  return <>{storybookUI}</>
}

from react-native.

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.

from react-native.

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.

from react-native.

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. :/

from react-native.

This issue should be resolved for good in 6.0 and I have some good news if you're waiting for that.

Now that all the issues in the first milestone have been completed I have released the first alpha.

I've also created an initial guide on how you can get setup with a fresh project. Migration guide to come in the future.

Find the guide here:
https://github.com/storybookjs/react-native/blob/next-6.0/v6README.md

from react-native.

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/> }
);

from react-native.

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 😄

from react-native.

@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.

from react-native.

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. :)

from react-native.

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

from react-native.

@4lun thanks for providing information and examples.

I will try out your example and see what comes up. In my projects I don't have issues with promises so I'm sure there is a way around this.

from react-native.

Thanks @dannyhw, I've ended up doing something similar to your suggestion, wrapping up all references to Storybook and conditionally loading it via an env flag.

The native module I'm working on shouldn't ever be utilised in a storybook context so this works quite well for the time being.

from react-native.

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

from react-native.

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

from react-native.

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

from react-native.

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.

from react-native.

@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.

from react-native.

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

from react-native.

@dannyhw any updates? has this been fixed?

from react-native.

@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

from react-native.

@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.

from react-native.

@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.

from react-native.

@4lun I've experimented a bit and the promise not returning only happens if you run the getStorybookUI({}) function call from what I can tell.
I'm still investigating so I don't have all the details, nor can I offer a proper solution yet.

Heres some stuff that may or may not be useful 🤷 .
In some cases you can get around this by not calling this function in the top of your app.tsx and instead have a function that will set up the UI when you need it.
like const getSBUI = () => getStorybookUI({asyncStorage: null});
then you can do something like

 if (showSB) {
    const SBUI = getSBUI();
    return <SBUI />;
  }

It's kind of a workaround for the case where you want to have storybook optionally render without it breaking other things in the app.

I realise it's not exactly the solution to your problem however it does make it possible to include storybook along side other react native code without breaking things when storybook isn't rendered.

Also it does confirm it's not the @react-native/storybook import. I'm still going to investigate further and see if I can find which line in the getStorybookUI function is causing this.

My theory on this is still that the polyfill issue comes from the storybook client api (found in the web storybook repo) which uses polyfills etc. I'm fairly confident in this because I've tried removing all polyfills from react-native storybook and it made no difference.

from react-native.

@4lun So I actually ended up finding what I think is the cause of the problem.

If you open up
node_modules/@storybook/addons/dist/index.js

and comment out line 11 //require("core-js/modules/es.promise"); like this then it starts working.

Again not an actual solution yet but from here I can try and figure out if there is a way around this.

I'm going to need help from someone on the storybook team probably because I'm not sure there is any reasonable way to fix this for react native without changes to the pollyfill on storybook addons

@ndelangen or @shilman if you have a moment maybe you can chime in on this? It's been brought up before but I wonder if maybe there is any way to avoid including corejs/es.promise? I looked on the latest version of addons and it still has this import but maybe this will be changing when internet explorer support gets dropped? Seems explorer is the only one without promise support.

from react-native.

A PR was created on the main storybook repository with a fix for future version of storybook. This means that this issue will be resolved in 6.0 of react native storybook. The PR enables provides a modern version of the distributed files, in your project you would add the following config resolverMainFields:['sbmodern', 'main'] and the storybook dependencies used would be without polyfils.

from react-native.

#159 (comment)

from react-native.

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", {
 

from react-native.

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}
     </>
  )
}
`` 

from react-native.

@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.

from react-native.

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.

from react-native.

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

from react-native.

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?

from react-native.

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.

from react-native.

@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

from react-native.

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

from react-native.

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?

from react-native.

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",

from react-native.

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

from react-native.

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.

from react-native.

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

from react-native.