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

ExpoGL on the Web: crash because of ref prop on GLView #10774

Closed
guyzmo opened this issue Oct 22, 2020 · 3 comments
Closed

ExpoGL on the Web: crash because of ref prop on GLView #10774

guyzmo opened this issue Oct 22, 2020 · 3 comments
Labels
GLView needs validation Issue needs to be validated Platform: web Using Expo in the browser stale

Comments

@guyzmo
Copy link

guyzmo commented Oct 22, 2020

🐛 Bug Report

Summary of Issue

Launching expo-pixi's Signature module in web mode crashes, but in native mode it works perfectly fine.
The issue is a difference of behaviour in the API of GLView regarding the ref prop.


N.B.: there's a second bug related to getContextAttributes() that can be solved in expo-pixi, I already documented that there, to fix it simply change in the Signature module that line:

    const getAttributes = context.getContextAttributes || (() => ({}));

into:

    const getAttributes = (() => ({}));

Environment - output of expo diagnostics & the platform(s) you're targeting

  Expo CLI 3.28.2 environment info:
    System:
      OS: Linux 5.8 Arch Linux
      Shell: 5.8 - /bin/zsh
    Binaries:
      Node: 14.11.0 - /usr/sbin/node
      Yarn: 1.22.5 - /usr/sbin/yarn
      npm: 6.14.7 - /usr/sbin/npm
    SDKs:
      Android SDK:
        API Levels: 15, 23, 24, 25, 26, 27, 28
        Build Tools: 23.0.1, 24.0.2, 25.0.1, 25.0.2, 25.0.3, 26.0.1, 26.0.2, 26.0.3, 27.0
.3, 28.0.3
        System Images: android-25 | Google APIs ARM EABI v7a, android-25 | Google APIs In
tel x86 Atom, android-28 | Google APIs Intel x86 Atom
        Android NDK: 14.1.3816874
    IDEs:
      Android Studio: 3.4 AI-183.6156.11.34.5692245
    npmPackages:
      @expo/webpack-config: ^0.12.16 => 0.12.37
      expo: ^39.0.0 => 39.0.3
      react: 16.13.1 => 16.13.1
      react-dom: 16.13.1 => 16.13.1
      react-native: https://github.com/expo/react-native/archive/sdk-39.0.0.tar.gz => 0.6
3.2
      react-native-web: ~0.13.7 => 0.13.14
    Expo Workflow: managed

Targeting the web platform.

Reproducible Demo

https://snack.expo.io/93GKJUA1c

That snack does not show the issue as is, as a fix needs to be applied to expo-pixi first (cf above).
Though in my own code, both errors are thrown together.

Steps to Reproduce

Take the code in the snack locally, and use a hacked version of expo-pixi to get through the first issue.
Then try the fix below.

Expected Behavior vs Actual Behavior

The issue is within the expo-pixi source code, in Signature.js:

      <GLView
        {...this.panResponder.panHandlers}
        onLayout={this.onLayout}
        key={'Expo.Signature-' + global.__ExpoSignatureId}
        ref={this.setRef}
        {...this.props}
        onContextCreate={this.onContextCreate}
      />

When you run the above on the ios/android platforms it works fine, but on the web platform it crashes, yelling:

Warning: GLView: `ref` is not a prop. Trying to access it will result in `undefined` being returned. If you need to access the same value within the child component, you should pass it as a different prop. (https://fb.me/react-special-props)
    in GLView (at Signature.js:186)
    in Signature (at ...)
    ...

The fix I found has been to remove the ref attribute, and simply refer to the GLView instance as a ref:

  render() {
    if (Platform.OS === 'web') {
      const glView = (
        <GLView
          {...this.panResponder.panHandlers}
          onLayout={this.onLayout}
          key={'Expo.Signature-' + global.__ExpoSignatureId}
          {...this.props}
          onContextCreate={this.onContextCreate}
        />
      );
      this.setRef(glView);
      return glView;
    }
    return (
        <GLView
          {...this.panResponder.panHandlers}
          onLayout={this.onLayout}
          ref={this.setRef}
          key={'Expo.Signature-' + global.__ExpoSignatureId}
          {...this.props}
          onContextCreate={this.onContextCreate}
        />
    )
  }

I believe the solution above is not right, because the ref= prop should behave the same on both web and native platforms. I believe the GLView component should expose a similar interface, and at worst not crash (or show a typescript error if the interface is different).
This is why I'm making an issue here about this and not on expo-pixi.

@guyzmo guyzmo added the needs validation Issue needs to be validated label Oct 22, 2020
guyzmo added a commit to guyzmo/expo-pixi that referenced this issue Oct 23, 2020
Fixing the ref prop access on the web platform

cf github issue expo/expo#10774
cf Github issue expo#72
@AdamJNavarro AdamJNavarro added GLView Platform: web Using Expo in the browser labels Oct 25, 2020
@guyzmo
Copy link
Author

guyzmo commented Sep 2, 2021

Hello 👋 I'm still using my own branch of expo-pixi to circumvent the exposed issue. It's about to be a year, and nobody cared about this ☹ Has anybody any advice/opinion about that, and what would be the best way to solve this?

@github-actions
Copy link
Contributor

github-actions bot commented Feb 3, 2022

This issue is stale because it has been open for 60 days with no activity. If there is no activity in the next 7 days, the issue will be closed.

@github-actions github-actions bot added the stale label Feb 3, 2022
@github-actions
Copy link
Contributor

This issue was closed because it has been inactive for 7 days since being marked as stale. Please open a new issue if you believe you are encountering a related problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GLView needs validation Issue needs to be validated Platform: web Using Expo in the browser stale
Projects
None yet
Development

No branches or pull requests

2 participants