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

Codegen: Add prepublish script to build Flow files #28645

Closed
wants to merge 1 commit into from
Closed

Codegen: Add prepublish script to build Flow files #28645

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ RNTester/Pods/*
# react-native-codegen
/ReactCommon/fabric/components/rncore/
/schema-rncore.json
/packages/react-native-codegen/lib

# Visual studio
.vscode
Expand Down
11 changes: 10 additions & 1 deletion packages/react-native-codegen/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,22 @@
"type": "git",
"url": "git@github.com:facebook/react-native.git"
},
"scripts": {
"build": "yarn clean && yarn flow-remove-types src/ -d lib/ -q && yarn flow-copy-source src/ lib/",
"clean": "rm -rf lib",
"prepublish": "yarn run build"
},
"license": "MIT",
"files": [
"src"
"lib"
],
"dependencies": {
"flow-parser": "^0.121.0",
"jscodeshift": "^0.7.0",
"nullthrows": "^1.1.1"
},
"devDependencies": {
"flow-copy-source": "^2.0.9",
"flow-remove-types": "^2.122.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately these two packages pull a lot of dependencies into our monorepo at FB that we don't really use internally so it would slow down our product developers quite a bit.

We use flow-remove-types at version 1.2.3. Can we use that for now and possibly not add flow-copy-source as it pulls in chokidar and a new version of Yargs etc.?

Copy link
Contributor Author

@empyrical empyrical Apr 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

flow-remove-types@1.2.3 runs into syntax errors with the files in Codegen:

src/CodegenSchema.js
 ↳ Syntax Error: Unexpected token (319:70)
   318:        components?: $ReadOnly<{[component: string]: ComponentShape, ...}>,

I could make a build script that does all this, and not need to use flow-remove-types or flow-copy-source at all, hopefully eliminating the version bump of flow-remove-types as I had no idea about the issues the version bump it had would have internally.

Metro, for example, has a nice script that could be adapted to this (builds the files with Babel, and copies over Flow sources as necessary)

https://github.com/facebook/metro/blob/b651e535cd0fc5df6c0803b9aa647d664cb9a6c3/scripts/build.js

If this sounds good, I could do this instead 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

@hramos has another diff/PR to just upgrade flow-remove-types. If you could rebase on top of that and then do some custom copying like Metro, I think that would be best.

}
}