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

[Lint] Update linter to use babel-lint and lint JSX/React directly #259

Closed
wants to merge 1 commit into from
Closed

[Lint] Update linter to use babel-lint and lint JSX/React directly #259

wants to merge 1 commit into from

Conversation

ide
Copy link
Contributor

@ide ide commented Mar 26, 2015

  • Upgraded eslint
  • Installed babel-eslint so that we can parse JSX and ES6
  • Installed eslint-react-plugin so we can lint JSX directly w/o a transform
  • no-comma-dangle was replaced with comma-dangle
  • space-unary-word-ops was replaced with space-unary-ops.

@ide ide mentioned this pull request Mar 26, 2015
Closed
@sahrens
Copy link
Contributor

sahrens commented Mar 31, 2015

@amasad Can we do this yet?

@amasad
Copy link
Contributor

amasad commented Apr 3, 2015

I think we need to update our eslint version first. Right?

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 7, 2015
@ide ide force-pushed the eslintrc branch 3 times, most recently from cfd2e3f to 09ab33c Compare April 15, 2015 21:00
@ide ide force-pushed the eslintrc branch 2 times, most recently from f7310d7 to e7b1894 Compare April 28, 2015 01:23
@ide
Copy link
Contributor Author

ide commented Apr 28, 2015

Probably the best way to approach this is to install eslint, babel-lint, and eslint-react-plugin directly so that React Native can have its own linter settings independent of any other project. My latest commit does this but there's an issue with the packager where it's finding conflicting @providesModule directives deep inside of node_modules (the "commoner" module's files) so that needs to be resolved before merging.

@amasad
Copy link
Contributor

amasad commented Apr 29, 2015

I had this on my list of things todo. I'll take a look soon!

@ide ide force-pushed the eslintrc branch 3 times, most recently from 1d5716b to 41be26d Compare May 13, 2015 23:03
@ide ide force-pushed the eslintrc branch 2 times, most recently from d5dcc41 to 538bff7 Compare May 16, 2015 06:09
@amasad
Copy link
Contributor

amasad commented May 18, 2015

Didn't forget about this. Switching to babel very soon and then we can easily use this.

@ide
Copy link
Contributor Author

ide commented May 18, 2015

I'll keep rebasing periodically so it's easy to merge and uses the latest eslint plugins.

@amasad
Copy link
Contributor

amasad commented May 20, 2015

Thanks @ide! We just switched to Babel, and we can safely merge this now (after testing internally, with phabricator etc)
@jaredly will take care of this.

@amasad amasad assigned jaredly and unassigned amasad May 20, 2015
jaredly
jaredly reviewed May 21, 2015
View reviewed changes
package.json
@@ -50,6 +50,7 @@
"debug": "~2.1.0",
"graceful-fs": "^3.0.6",
"image-size": "0.3.5",
"jest-cli": "^0.4.3",
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's unrelated iirc

Copy link
Contributor

Choose a reason for hiding this comment

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

jest should only be in devDependencies

@ide
[Lint] Update linter to use babel-lint and lint JSX/React directly
fefcb2d 
- Upgraded eslint
- Installed babel-eslint so that we can parse JSX and ES6
- Installed eslint-react-plugin so we can lint JSX directly w/o a transform
- `no-comma-dangle` was replaced with `comma-dangle`
- `space-unary-word-ops` was replaced with `space-unary-ops`.
@ide
Copy link
Contributor Author

ide commented May 21, 2015

Removed the unnecessary jest-cli entry from dependencies.

@ide ide changed the title [Lint] Update space-unary-ops rule in eslintrc (fixes Atom linter crash) [Lint] Update linter to use babel-lint and lint JSX/React directly May 21, 2015
@jaredly
Copy link
Contributor

jaredly commented May 22, 2015

Thanks!
I had to make some changes to get this to work for both open source and internal, and will show up at the next sync.

@ide
Copy link
Contributor Author

ide commented May 22, 2015

Sweet, looking forward to it.

This was referenced May 23, 2015
Closed
Closed
@ide
Copy link
Contributor Author

ide commented May 30, 2015

Yay! Thanks for landing it =)

@ide ide unassigned jaredly May 30, 2015
@ide ide closed this May 30, 2015
@ide ide deleted the eslintrc branch May 31, 2015 09:57
@amasad
Copy link
Contributor

amasad commented Jun 2, 2015

💃

mganandraj pushed a commit to mganandraj/react-native that referenced this pull request Mar 26, 2020
@HeyImChris
Remove a stale ref to a deprecated file (UIWebView) (facebook#259)
65f2bca 
* Remove a stale ref to a deprecated file

* remove more unneeded recovered files- remove libs by ref

* add back lib refs as we get build failures with them out

* fix up blank space issues
jfrolich pushed a commit to jfrolich/react-native that referenced this pull request Apr 22, 2020
@leostera @grabbou
Autonamespacing messes up module resolution (facebook#259)
997da7e
ayushjainrksh referenced this pull request in MLH-Fellowship/react-native Jul 2, 2020
@charpeni
Update keyboard example.md (#259)
f95c657 
* Update keyboard.md

* Update keyboard.md
@snyk-bot snyk-bot mentioned this pull request Dec 18, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ide @sahrens @amasad @jaredly @vjeux @facebook-github-bot