Skip to content

Use Babel for react-dev-utils - #2755

Closed
viankakrisna wants to merge 1 commit into
react:masterfrom
viankakrisna:react-dev-utils/babel
Closed

Use Babel for react-dev-utils#2755
viankakrisna wants to merge 1 commit into
react:masterfrom
viankakrisna:react-dev-utils/babel

Conversation

@viankakrisna

Copy link
Copy Markdown
Contributor

In response to #2638 (comment)

@viankakrisna

Copy link
Copy Markdown
ContributorAuthor

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

@viankakrisna

Copy link
Copy Markdown
ContributorAuthor

Ready for review 👍

Comment thread.eslintignore Outdated
my-app*
packages/react-scripts/template
packages/react-scripts/fixtures
packages/react-dev-utils/FileSizeReporter.js

Copy link
Copy Markdown
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?

Comment threadpackages/react-dev-utils/.gitignore Outdated
@@ -0,0 +1,19 @@
/FileSizeReporter.js

Copy link
Copy Markdown
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
Copy Markdown
ContributorAuthor

I forgot we can do wildcard for ignoring files 😄

@gaearongaearon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@gaearongaearon added this to the 1.0.11 milestone Jul 12, 2017
@viankakrisna

viankakrisna commented Jul 12, 2017

Copy link
Copy Markdown
ContributorAuthor

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

@viankakrisnaviankakrisna mentioned this pull request Jul 12, 2017
@viankakrisna

Copy link
Copy Markdown
ContributorAuthor

@gaearon rebased, anything more to get this merged?

Comment thread.eslintignore Outdated

Copy link
Copy Markdown

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
Copy Markdown
ContributorAuthor

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?

Comment threadpackages/react-dev-utils/.gitignore Outdated

Copy link
Copy Markdown

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
Copy Markdown
ContributorAuthor

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

@TimerTimer modified the milestones: 1.0.x, 1.0.11Aug 9, 2017
@viankakrisna
viankakrisnaforce-pushed the react-dev-utils/babel branch 3 times, most recently from 972cddb to 7c3875eCompareSeptember 10, 2017 09:03
@viankakrisna

Copy link
Copy Markdown
ContributorAuthor

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
Copy Markdown
Contributor

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

@viankakrisna

Copy link
Copy Markdown
ContributorAuthor

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

@viankakrisna
viankakrisnaforce-pushed the react-dev-utils/babel branch 2 times, most recently from ca4de75 to 1462aa1CompareNovember 12, 2017 16:41
@viankakrisna

Copy link
Copy Markdown
ContributorAuthor

@Timer@gaearon can we get this in?

remove build artifacts
move babel-cli babel-preset-react-app and cross-env to devDependencies
remove unused deps
@andriy-f

Copy link
Copy Markdown

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

@gaearon

Copy link
Copy Markdown
Contributor

@andriy-f which issues?

@gaearon

Copy link
Copy Markdown
Contributor

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
Copy Markdown
ContributorAuthor

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.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

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