Skip to content

Conversation

@brentvatne
Copy link
Collaborator

I thought it would be useful to help clear out references to no longer used styles and also catch typos on style names to have flow error when we try to access a style that isn't defined.

Example:

export default class AuthenticationScreen extends React.Component {
  render() {
    // This throws an error because `continer` is misspelled 
    return (
      <View style={styles.continer} /> 
    )
  }
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
  },
}
export default class AuthenticationScreen extends React.Component {
  render() {
    // This throws an error because no fancyContainer style is defined
    return (
      <View style={[styles.container, styles.fancyContainer]} /> 
    )
  }
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
  },
}

All credit goes to @jeffmo in this tweet: https://twitter.com/lbljeffmo/status/755179096271888385

Also included in the PR is some cleanup on styles that no longer exist, which were caught by this change :)

@ghost
Copy link

ghost commented Jul 19, 2016

By analyzing the blame information on this pull request, we identified @andreicoman11 and @nicklockwood to be potential reviewers.

@ghost ghost added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Jul 19, 2016
*/
create(obj: {[key: string]: any}): {[key: string]: number} {
var result = {};
create<T: Object, U>(obj: T): {[key:$Keys<T>]: number} {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no-unused-vars: 'U' is defined but never used

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove U?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay lint! I forgot to delete this extra type param while poking around

@vjeux
Copy link
Contributor

vjeux commented Jul 19, 2016

This is so good! I'm curious how many errors it'll generate in fb codebase :)

@vjeux
Copy link
Contributor

vjeux commented Jul 19, 2016

@facebook-github-bot shipit

@ghost ghost added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Jul 19, 2016
@ghost
Copy link

ghost commented Jul 19, 2016

Thanks for importing. If you are an FB employee go to Phabricator to review internal test results.

@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 19, 2016
@vjeux
Copy link
Contributor

vjeux commented Jul 19, 2016

107 Flow errors detected in root

Nice!

@skevy
Copy link
Contributor

skevy commented Jul 19, 2016

Super cool. Can't wait to use this technique everywhereeeeeee

create(obj: {[key: string]: any}): {[key: string]: number} {
var result = {};
create<T: Object, U>(obj: T): {[key:$Keys<T>]: number} {
var result: T = (({}: any): T);
Copy link
Contributor

@ide ide Jul 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think result should be of type {[key: $Keys<T>]: number}, this checks out on the Try Flow website:

function create<T: Object>(obj: T): {[key: $Keys<T>]: number} {
  var result: {[key: $Keys<T>]: number} = {};
  for (var key in obj) {
    StyleSheetValidation.validateStyle(key, obj);
    result[key] = ReactNativePropRegistry.register(obj[key]);
  }
  return result;
}

EDIT: this is a little prettier:

type Styles = {[key: string]: Object};
type StyleSheet<S: Styles> = {[key: $Keys<S>]: number};

function create<S: Styles>(styles: S): StyleSheet<S> {
  var styleSheet: StyleSheet<S> = {};
  for (var key in styles) {
    StyleSheetValidation.validateStyle(key, styles);
    styleSheet[key] = ReactNativePropRegistry.register(styles[key]);
  }
  return styleSheet;
}

https://flowtype.org/try/#0CYUwxgNghgTiAEA3W8DKAXAnhEqAWII6AalBAJbBTrkD2AdgFzwDyARgFbjoDcAUKEiwEyGPABKIKGHQA5auUQgACjFoAHSQHNyAZ3QxMzAN5948ODv0gYAClqduzdlxkBKZvQCuAWzY2AGj4AX34+LHUEDGwQXXgAXnhjAG0AaxAjeH0YcnotAF1nRxlQ8MxItCwcfEJ0AB4AFWZonF0APgSktIzmABIAaQzdRrbC+G8-G1LzPgAzL3oZOnp4MDhqEDrUZqrYttt9GN1mj0qYmqItjtNzUSzdi-Qd84JL1A7E42n4WdoxWzu6Uw8Fy9yObiSZnMZ2qrxIZEoCgYADpkBQqOhcLtbECAmDWm5+ND8bg4d1MPlOpJpHIFEpVBptHoDJhkZZmTYDrtdOT8oSocEoXB0F4YCtDrDavxBXwwAx9BYoAB3FqxTo3VYMdBQXI2ExQ8yzHAAD2YAEYguZgkFSnL6AqJWrEmspJjbDBlardPy+I7dMi7Tr6DZ+FyjgGtUG9fccnlCUA @

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch

@ghost ghost closed this in a564af8 Jul 19, 2016
@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 19, 2016
@vjeux
Copy link
Contributor

vjeux commented Jul 19, 2016

@yungsters fixed the internal call sites and pulled it in! Thanks

fadils pushed a commit to fadils/react-native that referenced this pull request Jul 21, 2016
… a stylesheet

Summary:
I thought it would be useful to help clear out references to no longer used styles and also catch typos on style names to have flow error when we try to access a style that isn't defined.

Example:

```javascript
export default class AuthenticationScreen extends React.Component {
  render() {
    // This throws an error because `continer` is misspelled
    return (
      <View style={styles.continer} />
    )
  }
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
  },
}
```

```javascript
export default class AuthenticationScreen extends React.Component {
  render() {
    // This throws an error because no fancyContainer style is defined
    return (
      <View style={[styles.container, styles.fancyContainer]} />
    )
  }
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
  },
}
```

All credit goes to jeffmo in this tweet: https://twitter.com/lbljeffmo/status/755179096271888385

Also included in the PR is some cleanup on styles that
Closes facebook#8876

Differential Revision: D3584983

Pulled By: yungsters

fbshipit-source-id: 0ee0e12ff3d976c137d932688e323c26690e0a52
rozele pushed a commit to microsoft/react-native-windows that referenced this pull request Jul 26, 2016
… a stylesheet

Summary:
I thought it would be useful to help clear out references to no longer used styles and also catch typos on style names to have flow error when we try to access a style that isn't defined.

Example:

```javascript
export default class AuthenticationScreen extends React.Component {
  render() {
    // This throws an error because `continer` is misspelled
    return (
      <View style={styles.continer} />
    )
  }
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
  },
}
```

```javascript
export default class AuthenticationScreen extends React.Component {
  render() {
    // This throws an error because no fancyContainer style is defined
    return (
      <View style={[styles.container, styles.fancyContainer]} />
    )
  }
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
  },
}
```

All credit goes to jeffmo in this tweet: https://twitter.com/lbljeffmo/status/755179096271888385

Also included in the PR is some cleanup on styles that
Closes facebook/react-native#8876

Differential Revision: D3584983

Pulled By: yungsters

fbshipit-source-id: 0ee0e12ff3d976c137d932688e323c26690e0a52
bubblesunyum pushed a commit to iodine/react-native that referenced this pull request Aug 23, 2016
… a stylesheet

Summary:
I thought it would be useful to help clear out references to no longer used styles and also catch typos on style names to have flow error when we try to access a style that isn't defined.

Example:

```javascript
export default class AuthenticationScreen extends React.Component {
  render() {
    // This throws an error because `continer` is misspelled
    return (
      <View style={styles.continer} />
    )
  }
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
  },
}
```

```javascript
export default class AuthenticationScreen extends React.Component {
  render() {
    // This throws an error because no fancyContainer style is defined
    return (
      <View style={[styles.container, styles.fancyContainer]} />
    )
  }
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
  },
}
```

All credit goes to jeffmo in this tweet: https://twitter.com/lbljeffmo/status/755179096271888385

Also included in the PR is some cleanup on styles that
Closes facebook#8876

Differential Revision: D3584983

Pulled By: yungsters

fbshipit-source-id: 0ee0e12ff3d976c137d932688e323c26690e0a52
mpretty-cyro pushed a commit to HomePass/react-native that referenced this pull request Aug 25, 2016
… a stylesheet

Summary:
I thought it would be useful to help clear out references to no longer used styles and also catch typos on style names to have flow error when we try to access a style that isn't defined.

Example:

```javascript
export default class AuthenticationScreen extends React.Component {
  render() {
    // This throws an error because `continer` is misspelled
    return (
      <View style={styles.continer} />
    )
  }
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
  },
}
```

```javascript
export default class AuthenticationScreen extends React.Component {
  render() {
    // This throws an error because no fancyContainer style is defined
    return (
      <View style={[styles.container, styles.fancyContainer]} />
    )
  }
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
  },
}
```

All credit goes to jeffmo in this tweet: https://twitter.com/lbljeffmo/status/755179096271888385

Also included in the PR is some cleanup on styles that
Closes facebook#8876

Differential Revision: D3584983

Pulled By: yungsters

fbshipit-source-id: 0ee0e12ff3d976c137d932688e323c26690e0a52
tungdo194 pushed a commit to tungdo194/rn-test that referenced this pull request Apr 28, 2024
… a stylesheet

Summary:
I thought it would be useful to help clear out references to no longer used styles and also catch typos on style names to have flow error when we try to access a style that isn't defined.

Example:

```javascript
export default class AuthenticationScreen extends React.Component {
  render() {
    // This throws an error because `continer` is misspelled
    return (
      <View style={styles.continer} />
    )
  }
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
  },
}
```

```javascript
export default class AuthenticationScreen extends React.Component {
  render() {
    // This throws an error because no fancyContainer style is defined
    return (
      <View style={[styles.container, styles.fancyContainer]} />
    )
  }
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
  },
}
```

All credit goes to jeffmo in this tweet: https://twitter.com/lbljeffmo/status/755179096271888385

Also included in the PR is some cleanup on styles that
Closes facebook/react-native#8876

Differential Revision: D3584983

Pulled By: yungsters

fbshipit-source-id: 0ee0e12ff3d976c137d932688e323c26690e0a52
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Import Started This pull request has been imported. This does not imply the PR has been approved.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants