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.

Sign up for GitHub

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

Jump to bottom

Fix the website build #12479

Closed
wants to merge 1 commit into from
Closed

Fix the website build #12479

wants to merge 1 commit into from

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Feb 20, 2017

118e883 broke autodoc generation because autodoc isn't smart enough to understand some patterns.

I'm working around it by using the same trick as this file was using previously: reassigning the class variable.

Similarly, using a property initializer produces bad output so I had to remove it:

screen shot 2017-02-20 at 19 00 36

cc @ericnakagawa @mkonicek for review

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. GH Review: review-needed labels Feb 20, 2017
@gaearon gaearon mentioned this pull request Feb 20, 2017
Closed
@gaearon
Fix the website build
fa62917 
118e883 broke autodoc generation because autodoc isn't smart enough to understand some patterns.
I'm working around it by using the same trick as this file was using previously: reassigning the class variable.
gaearon
gaearon commented Feb 20, 2017
View reviewed changes
Libraries/AppState/AppState.js
@@ -26,9 +26,6 @@ const invariant = require('fbjs/lib/invariant');
* AppState is frequently used to determine the intent and proper behavior when
* handling push notifications.
*
* This module depends on the native RCTAppState module. If you don't include it,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removing this from here since I don't want this to appear in open source documentation.

gaearon
gaearon commented Feb 20, 2017
View reviewed changes
Libraries/AppState/AppState.js
@@ -90,11 +87,12 @@ class AppState extends NativeEventEmitter {

_eventHandlers: Object;
currentState: ?string;
isAvailable: boolean = true;
isAvailable: boolean;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Autodoc doesn't understand property initializers.

gaearon
gaearon commented Feb 20, 2017
View reviewed changes
Libraries/AppState/AppState.js
@@ -213,10 +211,13 @@ class MissingNativeAppStateShim extends EventEmitter {
}
}

// Guard against missing native module by throwing on first method call.
// Keep the API the same so Flow doesn't complain.
const appState = RCTAppState
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Autodoc doesn't understand indirection via a variable.

@gaearon
Copy link
Collaborator Author

gaearon commented Feb 20, 2017

Note that this will still fail CI because of the other problems mentioned in #12450.

@bestander
Copy link
Contributor

Flow in e2e is still failing:
node_modules/react-native/Libraries/Experimental/flowtests/FlatList-flowtest.js:31
31: const data = [{
^ object literal. This type is incompatible with
19: item: {
^ object type
Property title is incompatible:
32: title: 6,
^ number. This type is incompatible with
20: title: string,
^^^^^^ string

@gaearon
Copy link
Collaborator Author

gaearon commented Feb 21, 2017

Flow in e2e is still failing:

See above comment:

Note that this will still fail CI because of the other problems mentioned in #12450.

I'm only fixing the part I previously touched, happy to have somebody else fix the Flow part (since I don't work on FlatList).

@bestander
Copy link
Contributor

bestander commented Feb 21, 2017 via email

@facebook-github-bot facebook-github-bot 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 Feb 21, 2017
@facebook-github-bot
Copy link
Contributor

@gaearon has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@gaearon gaearon deleted the workaround-autodoc branch February 21, 2017 17:14
GaborWnuk pushed a commit to GaborWnuk/react-native that referenced this pull request Feb 28, 2017
@gaearon
Fix the website build
01bfcd2 
Summary:
118e883 broke autodoc generation because autodoc isn't smart enough to understand some patterns.

I'm working around it by using the same trick as this file was using previously: reassigning the class variable.

Similarly, using a property initializer produces bad output so I had to remove it:

<img width="146" alt="screen shot 2017-02-20 at 19 00 36" src="https://cloud.githubusercontent.com/assets/810438/23138561/09ebb476-f7a0-11e6-8fad-92c5a01503c3.png">

cc ericnakagawa mkonicek for review
Closes facebook#12479

Differential Revision: D4591285

Pulled By: gaearon

fbshipit-source-id: 5884620a08874298b1b2c810e8fb769eba4e1199
dudeinthemirror pushed a commit to dudeinthemirror/react-native that referenced this pull request Mar 1, 2017
@gaearon @buildadm
Fix the website build
fb58ed7 
Summary:
118e883 broke autodoc generation because autodoc isn't smart enough to understand some patterns.

I'm working around it by using the same trick as this file was using previously: reassigning the class variable.

Similarly, using a property initializer produces bad output so I had to remove it:

<img width="146" alt="screen shot 2017-02-20 at 19 00 36" src="https://cloud.githubusercontent.com/assets/810438/23138561/09ebb476-f7a0-11e6-8fad-92c5a01503c3.png">

cc ericnakagawa mkonicek for review
Closes facebook#12479

Differential Revision: D4591285

Pulled By: gaearon

fbshipit-source-id: 5884620a08874298b1b2c810e8fb769eba4e1199
dudeinthemirror pushed a commit to dudeinthemirror/react-native that referenced this pull request Mar 1, 2017
@gaearon
Fix the website build
a07a4f1 
Summary:
118e883 broke autodoc generation because autodoc isn't smart enough to understand some patterns.

I'm working around it by using the same trick as this file was using previously: reassigning the class variable.

Similarly, using a property initializer produces bad output so I had to remove it:

<img width="146" alt="screen shot 2017-02-20 at 19 00 36" src="https://cloud.githubusercontent.com/assets/810438/23138561/09ebb476-f7a0-11e6-8fad-92c5a01503c3.png">

cc ericnakagawa mkonicek for review
Closes facebook#12479

Differential Revision: D4591285

Pulled By: gaearon

fbshipit-source-id: 5884620a08874298b1b2c810e8fb769eba4e1199
@snyk-bot snyk-bot mentioned this pull request Sep 16, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hramos hramos hramos approved these changes

@ericnakagawa ericnakagawa Awaiting requested review from ericnakagawa

@mkonicek mkonicek Awaiting requested review from mkonicek

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gaearon @bestander @facebook-github-bot @hramos