vitalets / react-native-extended-stylesheet

Extended StyleSheets for React Native
MIT License
2.93k stars 132 forks source link

result doesn't have Object prototype chain #101

Closed vreality64 closed 5 years ago

vreality64 commented 5 years ago

Steps to Reproduce

It's basic concept question. It could be bug or not based on situation. There is some console warning when use React Native's FlatList component with created style sheet.

const styles = {
  // I know it's not root level, However it's not bad first start step.
  container: EStylesheet.create({
    backgroundColor: 'white'
  })
}

const StyledList = () => (
  <FlatList
    contentContainerStyle={styles.container}  // it's also happen 'style' props
    // ... other FlatList props
  />
)

Expected Behavior

It shouldn't warn anything.

Actual Behavior

there are console warning in development mode

ExceptionsManager.js:84 Warning: Failed prop type: two.hasOwnProperty is not a function
    in ScrollView (at VirtualizedList.js:1028)
    in VirtualizedList (at FlatList.js:654)
    ...

Show the code

This code makes console warning because Object.create(null) doesn't have prototype chain of Object. Is this intended?

this.result = Object.create(null)

Environment

├─ react-native-extended-stylesheet@0.10.0
└─ react-native@0.56.0

Suggestions

Object.create(null) is internally okay as usage of map. However it has underlying possible error about Object prototype chain which is not easily noticed. I thinks it'd better to having prototype chain of Object in result of create function.

vitalets commented 5 years ago

@vreality64 I think it was done to avoid accidental access to style props that do not exist. But actually i agree with you - it can give more problems than profit. Could you please check is RN original stylesheet has prototype chain or not? If it has - we should definitely be the same. Thank you!

vreality64 commented 5 years ago

@vitalets As you can see, StyleSheet.create({ ... }) actually doesn't do anything on prototype. RN original stylesheet has prototype chain, because they return original object.

Here are code sandbox link.

vitalets commented 5 years ago

@vreality64 Thank you for the research! Will fix it and release.

vreality64 commented 5 years ago

@vitalets In my opinion, simplest and good solution is just changing sheet.getResult() to Object.assign({}, sheet.getResult()) in code. It's already ES6 base. 😃