Skip to content

Use Babel for react-dev-utils #2755

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
Closed
wants to merge 1 commit into from
Closed

Use Babel for react-dev-utils #2755

wants to merge 1 commit into from

Conversation

@viankakrisna
Copy link
Contributor

@viankakrisna viankakrisna commented Jul 9, 2017

In response to #2638 (comment)

@viankakrisna
Copy link
Contributor Author

viankakrisna commented Jul 10, 2017

I'm lost on how to make lerna works when there's a build dependency between packages -.-' will sleep on it...

@viankakrisna
Copy link
Contributor Author

viankakrisna commented Jul 12, 2017

Ready for review 👍

gaearon
gaearon reviewed Jul 12, 2017
View reviewed changes
.eslintignore Outdated
my-app*
packages/react-scripts/template
packages/react-scripts/fixtures
packages/react-dev-utils/FileSizeReporter.js
Copy link
Contributor

@gaearon gaearon Jul 12, 2017

Choose a reason for hiding this comment

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

This seems unfortunate. Can we just ignore every file in the root folder?

gaearon
gaearon reviewed Jul 12, 2017
View reviewed changes
packages/react-dev-utils/.gitignore Outdated
@@ -0,0 +1,19 @@
/FileSizeReporter.js
Copy link
Contributor

@gaearon gaearon Jul 12, 2017

Choose a reason for hiding this comment

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

Same here, I'd prefer we just ignore all JS files in the root folder.

@viankakrisna
Copy link
Contributor Author

viankakrisna commented Jul 12, 2017

I forgot we can do wildcard for ignoring files 😄

gaearon
gaearon approved these changes Jul 12, 2017
View reviewed changes
Copy link
Contributor

@gaearon gaearon left a comment

Choose a reason for hiding this comment

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

LGTM

@gaearon gaearon added this to the 1.0.11 milestone Jul 12, 2017
@viankakrisna
Copy link
Contributor Author

viankakrisna commented Jul 12, 2017 •
edited
Loading

Somehow, Travis is hanging on node 8's kitchensink. Maybe need a restart?

@viankakrisna viankakrisna mentioned this pull request Jul 12, 2017
Closed
@viankakrisna viankakrisna force-pushed the react-dev-utils/babel branch from bdbb216 to 2237356 Compare August 5, 2017 15:21
@viankakrisna
Copy link
Contributor Author

viankakrisna commented Aug 5, 2017

@gaearon rebased, anything more to get this merged?

miraage
miraage reviewed Aug 7, 2017
View reviewed changes
.eslintignore Outdated
Copy link

@miraage miraage Aug 7, 2017

Choose a reason for hiding this comment

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

No newline here. Is it intended?

Copy link
Contributor Author

@viankakrisna viankakrisna Aug 23, 2017

Choose a reason for hiding this comment

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

screen shot 2017-08-23 at 8 51 37 pm 1
have it here, I think GitHub truncate it?

miraage
miraage reviewed Aug 7, 2017
View reviewed changes
packages/react-dev-utils/.gitignore Outdated
Copy link

@miraage miraage Aug 7, 2017

Choose a reason for hiding this comment

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

No newline here. Is it intended? [2]

Copy link
Contributor Author

@viankakrisna viankakrisna Aug 23, 2017

Choose a reason for hiding this comment

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

screen shot 2017-08-23 at 8 54 10 pm
same as above

@Timer Timer modified the milestones: 1.0.x, 1.0.11 Aug 9, 2017
@viankakrisna viankakrisna force-pushed the react-dev-utils/babel branch from 2237356 to 0aff591 Compare August 23, 2017 13:48
@viankakrisna viankakrisna force-pushed the react-dev-utils/babel branch 3 times, most recently from 972cddb to 7c3875e Compare September 10, 2017 09:03
@viankakrisna
Copy link
Contributor Author

viankakrisna commented Sep 10, 2017

screen shot 2017-09-11 at 1 08 32 am

seems like related to http://codetunnel.io/npm-5-changes-to-npm-link/

@Timer
Copy link
Contributor

Timer commented Sep 10, 2017

Do we need to switch away from npm@5 for CI?

@viankakrisna
Copy link
Contributor Author

viankakrisna commented Sep 10, 2017

need to find out why exactly it's green on #3104 and #3026 but not here

@viankakrisna viankakrisna mentioned this pull request Sep 10, 2017
Closed
@viankakrisna viankakrisna force-pushed the react-dev-utils/babel branch 2 times, most recently from 2816dc1 to f36b6e6 Compare September 14, 2017 17:42
@viankakrisna viankakrisna mentioned this pull request Oct 18, 2017
Closed
@viankakrisna viankakrisna force-pushed the react-dev-utils/babel branch 2 times, most recently from ca4de75 to 1462aa1 Compare November 12, 2017 16:41
@viankakrisna
Copy link
Contributor Author

viankakrisna commented Nov 19, 2017

@Timer @gaearon can we get this in?

@viankakrisna viankakrisna mentioned this pull request Jan 3, 2018
Closed
@viankakrisna viankakrisna force-pushed the react-dev-utils/babel branch from 1462aa1 to 7ae04c4 Compare January 11, 2018 16:21
@viankakrisna
use babel for react-dev-utils
f18b828 
remove build artifacts

move babel-cli babel-preset-react-app and cross-env to devDependencies

remove unused deps
@viankakrisna viankakrisna force-pushed the react-dev-utils/babel branch from 7ae04c4 to f18b828 Compare January 11, 2018 16:25
@andriy-f
Copy link

andriy-f commented Jan 13, 2018

It would be nice to merge this, it holds several other important issues

@gaearon
Copy link
Contributor

gaearon commented Jan 13, 2018

@andriy-f which issues?

@gaearon
Copy link
Contributor

gaearon commented Jan 13, 2018

I’m not 100% convinced we need to increase complexity here because we can tell users Node 8 is required, figure out our node_modules compilation story, and then in both cases ES6 will work fine. So maybe we should focus on that instead.

@viankakrisna
Copy link
Contributor Author

viankakrisna commented Jan 13, 2018

Yea, the idea is for this PR is that we want to extract service worker code to react-dev-utils, but I see what you mean.

@lock lock bot locked and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@miraage miraage miraage left review comments

@gaearon gaearon gaearon approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CLA Signed tag: internal

Projects

None yet

Milestone

1.0.x

Development

Successfully merging this pull request may close these issues.

6 participants

@viankakrisna @Timer @andriy-f @gaearon @miraage @facebook-github-bot