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

Minification causes issues in react-native release build #182

Closed
xzilja opened this issue Jun 17, 2018 · 5 comments
Closed

Minification causes issues in react-native release build #182

xzilja opened this issue Jun 17, 2018 · 5 comments

Comments

@xzilja
Copy link

xzilja commented Jun 17, 2018

Do you want to request a feature or report a bug?
Report a bug

What is the current behavior?
Issue is related to react-native, the way it works currently is in development mode, but once I run release, I am getting app crush at launch-screen. I pinpointed the issue to be related with code snippet below and uglify-es (I believe this is what is used for minification?)

Before opening this issue I asked for support over at uglify js github, where I've been told that uglify-es is no longer maintained.

I also opened this stack overflow issue that goes into the issue and what I am trying to achieve in depth: https://stackoverflow.com/questions/50898846/methods-for-react-hoc-that-takes-in-and-returns-any-amount-of-given-parameters

If the current behavior is a bug, please provide the steps to reproduce and a minimal repository on GitHub that we can yarn install and yarn test.

Essentially, something in this Higher Order Component fails when minified for release build and I am not sure how to fix it besides asking here, i.e. maybe we need to update to using uglify-js as this is what's being maintained?

// @flow
import React from 'react'
import { Subscribe } from 'unstated'
import type { Container } from 'unstated'

const getStoreAsProps = (storeArr: Container<any>[]) => {
  const storeProps = {}
  storeArr.map(value => (storeProps[value.constructor.name] = value))
  return storeProps
}

const withStore = (...args: any[]) => (Element: any) => () => (
  <Subscribe to={[...args]}>{(...args) => <Element {...getStoreAsProps(args)} />}</Subscribe>
)

export default withStore

What is the expected behavior?
It should work fine in dev and release (minified) builds

Please provide your exact Metro configuration and mention your Metro, node, yarn/npm version and operating system.

Exact configuration is one used in react-native@0.55.4, jusdging by my lock file it includes:
metro
metro-babylon7@0.30.2
metro-cache@0.30.2
metro-core@0.30.2
metro-minify-uglify@0.30.2
metro-resolver@0.30.2
metro-source-map@0.30.2
metro@^0.30.0

node: 10.4.0
yarn: 1.7.0
OS: macOS High Sierra

@rafeca
Copy link
Contributor

rafeca commented Jun 18, 2018

@iljadaderko the issue that you're facing is that you're accessing value.constructor.name and I guess that you're assuming in your code that this will have the same value as some Container class. During minification, class names get renamed to be shorter, so your code will probably break.

Instead of doing value.constructor.name, I suggest using some unique identifier from that class (you can create a name property on each Container for example), this will play nice with the minifier.

Hope that this helps 😃

@rafeca
Copy link
Contributor

rafeca commented Jun 18, 2018

I'm going to close this issue, feel free to reopen if my suggestion does not work well

@rafeca rafeca closed this as completed Jun 18, 2018
@xzilja
Copy link
Author

xzilja commented Jun 18, 2018

@rafeca Wow, this is so cool! I didn't realise, thank you for the explanation, it helped and is working now 🙌

@mtnt
Copy link

mtnt commented Nov 3, 2019

@rafeca your suggestion works well, but it`s just a workaround. Your offer to make something explicitly instead of using already separated by name code elements.

For example, I have an "abstract" class Screen that had a static method getName() { return this.name; }. And each screen extends the Screen. I`m using the getName for defining screens for the react-native-navigation and... I got an exception =(
I don`t wanna add another one constant for each screen, coz I already did - the class name.

@akinncar
Copy link

akinncar commented Nov 7, 2019

@iljadaderko the issue that you're facing is that you're accessing value.constructor.name and I guess that you're assuming in your code that this will have the same value as some Container class. During minification, class names get renamed to be shorter, so your code will probably break.

Instead of doing value.constructor.name, I suggest using some unique identifier from that class (you can create a name property on each Container for example), this will play nice with the minifier.

Hope that this helps

Thank you so much, after three days, you saved my life.
I switched my constructor calls to an atributte i created in component like you said.

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

4 participants