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

Use Babel for react-dev-utils #2755

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

In response to #2638 (comment)

@viankakrisna
Copy link
Contributor Author

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

Ready for review 👍

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

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

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

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
Copy link
Contributor Author

@gaearon rebased, anything more to get this merged?

miraage
miraage reviewed Aug 7, 2017
View reviewed changes
.eslintignore
@@ -3,3 +3,4 @@ build
my-app*
packages/react-scripts/template
packages/react-scripts/fixtures
packages/react-dev-utils/*.js
Copy link

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

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
@@ -0,0 +1 @@
/*.js
Copy link

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

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 3 times, most recently from 972cddb to 7c3875e Compare September 10, 2017 09:03
@viankakrisna
Copy link
Contributor Author

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

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

@Timer @gaearon can we get this in?

@viankakrisna viankakrisna mentioned this pull request Jan 3, 2018
Closed
@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
@andriy-f
Copy link

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

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

@miraage miraage miraage left review comments

@gaearon gaearon gaearon approved these changes

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.

None yet
6 participants
@viankakrisna @Timer @andriy-f @gaearon @miraage @facebook-github-bot