react-native-global-props icon indicating copy to clipboard operation
react-native-global-props copied to clipboard

Add tests

Open pke opened this issue 4 years ago • 2 comments

This project lacks some tests to verify the functionality. Do you have any plans to add tests or would accept a PR?

Case in point:

export const setCustomText = customProps => {
  const TextRender = Text.render
  const initialDefaultProps = Text.defaultProps
  Text.defaultProps = {
    ...initialDefaultProps,
    ...customProps
  }
  Text.render = function render(props) {
    let oldProps = props
    props = { ...props, style: [customProps.style, props.style] }
    try {
      return TextRender.apply(this, arguments)
    } finally {
      props = oldProps
    }
  }
}

This code can not work. It calls TextRender.apply with the original arguments, it assigns new props ignoring the Text.defaultProps.style and then assigns the props back in the finally block to no effect.

Tests would discover this faulty behaviour.

A working version of the above code seems to be:

const setCustomText = (customProps) => {
  Text.defaultProps = {
    ...Text.defaultProps,
    ...customProps,
  }
  const orgRender = Text.render
  Text.render = function render(props:TextProps, ref:Ref<Text>) {
    const style = Array.isArray(props.style) ? props.style : [props.style]
    props = {
      ...props,
      style: [Text.defaultProps.style, ...style],
    }
    return orgRender.call(this, props, ref)
  }
}

pke avatar May 21 '20 00:05 pke

Thanks for creating this issue. I would definitely accept a PR with added tests.

Ajackster avatar May 22 '20 02:05 Ajackster

Thanks for your feedback! I wonder how could the current code ever work? Or was just nobody using it?

I also wonder why RN does not offer this by default. I tried to search the issues over there but no-one seems to be missing setting a font globally for all Text elements. Everyone seems to be happy to derive a custom Text component for their apps that no one developer of this app should ever forget to use. And then there are 3rd party components that render just the default RN Text component and sometimes don't give you the styling options to change that.

Really confusing why this is not part of the default RN lib.

pke avatar May 22 '20 14:05 pke