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.

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

Adds support for extra source extensions via CLI arguments #11932

Closed
wants to merge 15 commits into from

Conversation

orta
Copy link
Contributor

@orta orta commented Jan 16, 2017

What existing problem does the pull request solve?

Hi there, I want to use TypeScript with React Native ( orta/systems-theory#1 )

All of the current techniques for this resolves around a two-step building process, wherein you have two toolchains running and watching for changes.

For example you would have TypeScript parsing a file, which would drop a JS file into a build folder, where react-packager would then pick it up and do the usual babel work. I'd like to simplify and unify that toolchain process.

I felt that this was reasonable because of the getTransformModulePath option inside the CLI, a developer can use a custom transform function to handle different source types before handing off the results to Babel to ensure the rest of the process acts like it does right now.

Test plan (required)

I have used this locally, and amended a test to use the updated syntax, but with some guidance I can add some more tests to this PR 👍

Code formatting

🥇 (I think...)

Sidenote

I expected that putting a rn-cli.config.js inside the root of my project would have it's variables reflected by default, but instead I need to run with --config rn-cli.config.js - is this expected behavior?

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot facebook-github-bot 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 Jan 16, 2017
@orta
Copy link
Contributor Author

orta commented Jan 16, 2017

Note: This doesn't attempt to handle some of the trickier source map issues, I think as long as I have access to a transform module function, I should be able to handle that.

@hramos
Copy link
Contributor

hramos commented Jan 19, 2017

Thanks for the PR! Will wait for @davidaurelio to take a look.

@@ -92,6 +92,11 @@ module.exports = {
parse: (val) => val.split(','),
default: (config) => config.getProjectRoots(),
}, {
command: '--sourceExts [list]',
description: 'Specify any source extentions to be used by the packager, defaults to js',
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this can be made slightly more helpful by mentioning that this can be used in combination with --transformer to process other file types?

orta referenced this pull request in artsy/emission Jan 30, 2017
- Removes flowconfig
- Adds tsconfig
- Renames all .js files to .tsx
@orta
Copy link
Contributor Author

orta commented Feb 7, 2017

@davidaurelio - I'm happy to rebase, and make changes if the conceptual idea is 👍 ( that adding a CLI flag / config setting is a good way to do this? )

Martin Konicek added 4 commits February 8, 2017 11:46
Summary:
Currently it is not trivial for people to get started with React Native. `react-native init MyApp` just creates a simple app with a single screen. People have to spend time figuring out how to add more screens, or how to accomplish very basic tasks such as rendering a list of data or handling text input.

Let's add an option: `react-native init --template navigation` - this creates a "starter" app which can be easily tweaked into the actual app the person wants to build.

**Test plan (required)**

- Checked that 'react-native init MyApp' still works as before:

<img width="487" alt="screenshot 2017-02-02 16 56 28" src="https://cloud.githubusercontent.com/assets/346214/22559344/b2348ebe-e968-11e6-9032-d1c33216f490.png">

<img width="603" alt="screenshot 2017-02-02 16 58 04" src="https://cloud.githubusercontent.com/assets/346214/22559370/c96a2ca6-e968-11e6-91f7-7afb967920fc.png">

- Ran 'react-native init MyNavApp --template'. This prints the available templates:

```
$ react-native init MyNavApp
Closes facebook#12170

Differential Revision: D4516241

Pulled By: mkonicek

fbshipit-source-id: 8ac081157919872e92947ed64ea64fb48078614d
Summary:
Update the template that will be used by `react-native init --template navigation`. The goal is to make it easier for people to get started by demonstrating a few very basic concepts such as navigation, lists and text input.

**Test plan (required)**

<img src="https://cloud.githubusercontent.com/assets/346214/22697898/ced66f52-ed4a-11e6-9b90-df6daef43199.gif" alt="Android Example" height="800" style="float: left"/>

<img src="https://cloud.githubusercontent.com/assets/346214/22697901/cfeab3e4-ed4a-11e6-8552-d76585317ac2.gif" alt="iOS Example" height="800"/>
Closes facebook#12260

Differential Revision: D4521758

Pulled By: mkonicek

fbshipit-source-id: d7d9e481dd3373917ac68ec9169b9ac3267547a9
Reviewed By: mkonicek

Differential Revision:
D4522116
Ninja: oss only

fbshipit-source-id: d17e0e8badcfe97eaea092e5d5274e02904a534d
ide and others added 5 commits February 8, 2017 17:51
React 15.4.x has been out for awhile. Test Plan: open UI Explorer, go through the various screens and look for warnings or errors.
Summary:
Fixes facebook#11272
Fixes facebook#11572
Fixes facebook#11781

The main changes here are:

* This depends on the latest CocoaPods (1.2.0). It’s currently in RC, but if I’m not mistaken a proper release is expected soon. /cc dantoml
* Adds required header search paths for the jschelpers and cxxreact subspecs.
* Makes the jschelpers and cxxreact headers private to building React Native, not visible to the user’s project.
* It uses the canonical upstream Yoga v1.0.0 podspec: https://github.com/facebook/yoga/blob/master/Yoga.podspec
* Consistent styling.

I have been able to get our app to build again using this artsy/emission#437. The spec has some warnings, but otherwise fully passes lint.

rh389 sjmueller Could you please test with your projects?
Closes facebook#12089

Differential Revision: D4518605

fbshipit-source-id: ecf86232d8b1af52d139eadd1acc10f5c1d42c29
@alloy
Copy link
Contributor

alloy commented Feb 23, 2017

Rebased this on top of 0.42.0-rc.3 for now.

@alloy
Copy link
Contributor

alloy commented Feb 25, 2017

ide and others added 3 commits February 28, 2017 11:40
Summary:
In my RN checkout, I use "upstream" as my remote instead of "origin" -> this lets me run `scripts/bump-oss-version.js --remote upstream 0.41.1` for example.

Also made the script executable so we don't need to put `node` in front of it, and updated the Releases.md doc.
Closes facebook#12230

Differential Revision: D4515070

Pulled By: mkonicek

fbshipit-source-id: f218a6b77959588ee5f625b8589ac080dd010034
@ds300
Copy link

ds300 commented Apr 20, 2017

Is there still interest in having this feature? I wouldn't mind doing the work to bring it up to date.

@43081j
Copy link

43081j commented Apr 20, 2017

unless you can suggest another, non-hacky way to add typescript to the build process (without just having a separate build workflow), i would say this is still very useful.

@ds300
Copy link

ds300 commented Apr 20, 2017

Yes. I suppose I'm asking for someone at FB to express interest in this feature though. No point putting the work in if it is just going to be ignored.

@43081j
Copy link

43081j commented Apr 23, 2017

is there any recommended work-around for now?

i have been struggling to get typescript as part of the build process. right now im having to run two separate build tasks each time (to compile TS, then react-native run). I did try using asset extensions config but it doesn't seem to work.

@ds300
Copy link

ds300 commented Apr 23, 2017

I think, maybe depending on which version of react-native you use, assetExts + custom transformer should work as long as you require TypeScript files with their extensions like import {blah} from 'blub.tsx'

You could also try using https://github.com/callstack-io/haul, which is very easy to set up for TypeScript.

Failing that, the only option I know of is to fork and patch react-native.

@cpojer
Copy link
Contributor

cpojer commented Apr 25, 2017

Alright, I think it is time to ship this thing. @orta would you be able to make this change more incremental with many smaller PRs? As it is, it is quite hard to review and ship with confidence; it also doesn't add any tests ;)

@ds300
Copy link

ds300 commented Apr 25, 2017

@cpojer This pr is out-of-date and so the diff with master looks funky. The actual changes made by @orta are fairly minimal: 6dca2ae

But they still need to be re-done for 0.43 since some significant changes have been made to the packager code. Also, yes, needs tests :)

As mentioned before, I'm happy to do this too, if @orta is busy.

@orta
Copy link
Contributor Author

orta commented Apr 25, 2017

Awesome, I'm basically OOO for the rest of this week - so @ds300 you're welcome to update it ( I just gave you commit access to my fork, or you can create your own PR)

@cpojer
Copy link
Contributor

cpojer commented Apr 25, 2017

Yep, rebasing and making it minimal/incremental + adding tests seems like a good first start. CC me.

@ds300
Copy link

ds300 commented Apr 27, 2017

@orta I made a new PR to avoid the mess, since so much had changed. It's over at #13689

@cpojer
Copy link
Contributor

cpojer commented Apr 27, 2017

I'll close this one and take a look at the other one tomorrow :)

@cpojer cpojer closed this Apr 27, 2017
@hramos hramos added the p: Microsoft Partner: Microsoft label Aug 9, 2019
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. p: Microsoft Partner: Microsoft
Projects
None yet
Development

Successfully merging this pull request may close these issues.