Skip to content

ESlint 2.0 #92

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

Closed
wants to merge 36 commits into from
Closed

ESlint 2.0 #92

wants to merge 36 commits into from

Conversation

pbrisbin
Copy link
Contributor

@pbrisbin pbrisbin commented May 26, 2016

This is the branch we'll be deploying the eslint-2 channel from as we begin
initial testing of that image. It's #79 with some additional changes (see
individual commits) and anything we identify and address during testing.

More documentation about this change is on the way, but for anyone who
sees this PR and wants to use it, the engine can be opted into with:

engines:
  eslint:
    enabled: true
    channel: eslint-2

Dan Rumney and others added 4 commits May 26, 2016 17:40
- Add access to ESLint docs
- Add support for parametrized Docker builds, building different
  versions of the documentation
- Automatically detect which version of the docs we should be including
Fixes:

    npm WARN eslint-config-airbnb@6.2.0 requires a peer of eslint@^2.4.0 but none was installed.
    npm WARN codeclimate-eslint@0.0.3 No license field.
    Cloning into 'eslint'...
    npm ERR! peer dep missing: eslint@^2.4.0, required by eslint-config-airbnb@6.2.0
    npm ERR! code 1
    error: pathspec 'vnull' did not match any file(s) known to git.
Fixes:

    /usr/src/app/node_modules/meld/meld.js:67
                      if (typeof target[pointcut] === 'function') {
                                        ^

    TypeError: Cannot read property 'loadPackage' of undefined
        at meld (/usr/src/app/node_modules/meld/meld.js:67:23)
        at Function.around (/usr/src/app/node_modules/meld/meld.js:436:12)
        at patcher (/usr/src/app/lib/eslint-patch.js:21:8)
        at Object.<anonymous> (/usr/src/app/bin/eslint.js:11:44)
        at Module._compile (module.js:397:26)
        at Object.Module._extensions..js (module.js:404:10)
        at Module.load (module.js:343:32)
        at Function.Module._load (module.js:300:12)
        at Function.Module.runMain (module.js:429:10)
        at startup (node.js:139:18)
@pbrisbin pbrisbin changed the title Eslint 2.0 ESlint 2.0 May 26, 2016
dblandin added 8 commits May 27, 2016 15:34
```
eslint@2.4.0               -> eslint@2.10.2
eslint-config-airbnb@6.0.2 -> eslint-config-airbnb@9.0.1
eslint-plugin-babel@2.1.1  -> eslint-config-airbnb@3.2.0
eslint-plugin-react@4.0.0  -> eslint-config-react@5.1.1
```
…lugin-config

eslint-2: Support standard style ESLint shared config and plugin
…le-config

eslint-2: Update CircleCI config to use patrick
@chibicode
Copy link

@jpignata @dblandin could we add eslint-config-standard-jsx to package.json as well? This is required for React developers (and this is included when you install standard).

@dblandin
Copy link
Contributor

dblandin commented Jun 15, 2016

@chibicode Sounds good to me! I'll open up a PR to pull that in.

Many users using [Standard Style][] also use these configuration packages when
writing React/JSX.

[Standard Style]: https://github.com/feross/standard

This commit vendors the [eslint-config-standard-react] and
[eslint-config-standard-jsx] packages.

[eslint-config-standard-react]: https://github.com/feross/eslint-config-standard-react
[eslint-config-standard-jsx]: https://github.com/feross/eslint-config-standard-jsx
@dblandin
Copy link
Contributor

Hey @chibicode,

I just pushed out a new engine image to the eslint-2 channel with support for the eslint-config-standard-react and eslint-config-standard-jsx configs.

Let me know if it works for you!

@chibicode
Copy link

@dblandin great, thank you!

pbrisbin and others added 9 commits July 15, 2016 10:13
Add eslint-config-google as available plugin
Add support for ESLint plugin Flowtype
eslint-2: Vendor eslint-plugin-angular package
By default we filter out minified files that are identifiable as minified from
their filename (e.g. *.min.js), but many projects have minified files that
don't match those patterns, and these files can cause ESLint to segfault in a
container environment (possibly due to memory pressure).

This adds a new heuristic filter based on line length that filters out files
that appear to be minified to prevent them being analyzed. For any matching
files, a log message is sent to stderr notifying that the file is being
skipped.
Heuristically filter out minified files
Aaron Craig and others added 11 commits August 11, 2016 16:49
Related to #111, a Segfault occurs when we hammer STDOUT with issues.
Switching from alpine to a debian-based image seems to behave better in
this scenario.
Switch base image to avoid Segfault
We're a bit behind on these. Flowtype in particular has new rules we
weren't supporting.

Looks like we hadn't actually filled in the shrinkwrap file before now,
either, so that's done.
- Update eslint configuration
- Reduce complexity via temporary variable
- Consistent brace style

Extracted from #109
Makes it possible to use ignorePath from the CLIEngine. Useful when you
are already ignoring the files you don't want to lint in different file
(.e.g .gitignore).
hapi eslint config is based on eslint-config-hapi
and eslint-plugin-hapi packages

module.exports = function patcher(eslint) {

meld.around(eslint.CLIEngine, 'loadPlugins', function(joinPoint) {

Choose a reason for hiding this comment

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

Is this patch working as intended? I arrived here because I'm seeing Failed to load plugin errors for an unsupported plugin, but I think this patch is intended to avoid loading unsupported plugins?

I'm questioning whether this does anything because it appears to me that eslint.CLIEngine has not had a loadPlugins method since this commit, and that commit has been around since ESLint 2.0.0?

Copy link
Contributor Author

@pbrisbin pbrisbin Aug 25, 2016

Choose a reason for hiding this comment

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

This patch is not working as intended. It was at the time it was written, but (as you noted) that's no longer the case given the current internals of ESLint.

We're evaluating if we want to accept unsupported plugins causing errors like that, or try to bring back the logic intended to replace that with ignoring them. Unfortunately, this meld approach doesn't appear viable on the new internals, so it would require moving back to our own fork of ESLint itself.

In the meantime though, we should probably remove this.

EDIT: It may not have even worked when it was written. The history is uncertain here.

chadxz and others added 2 commits August 29, 2016 21:05
eslint-config-airbnb-base is a transitive dependency of
eslint-config-airbnb, but it is useful in its own right and should be
included at the top level such that it has proper support in the eslint
engine.
Add eslint-config-airbnb-base as a top-level dep
@dblandin
Copy link
Contributor

Hey everyone!

Development for the eslint-2 channel has moved to the channel/eslint-2 branch. That branch won't have a tracking PR. Instead, new PRs targeting the eslint-2 channel should be opened with a base of channel/eslint-2.

Also, if you're interested, we just released an eslint-3 channel!

Thanks!

@dblandin dblandin closed this Sep 12, 2016
@dblandin dblandin deleted the pb-eslint-2 branch September 12, 2016 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.