Skip to content

Merge with eslint-plugin-prettify #21

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

Merged
zertosh merged 9 commits into from
May 16, 2017
Merged

Merge with eslint-plugin-prettify #21

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3e983f1
Chore: Use eslint-plugin-prettier without symlink
zertosh May 14, 2017
bb912f4
Chore: Lint .eslintrc.js via CLI automatically
zertosh May 14, 2017
e313bdd
Chore: Update remaining repo links
zertosh May 14, 2017
3d757e6
New: Replace implementation with line-by-line reporting (fixes #17)
zertosh May 14, 2017
8d9b6e7
Fix: Use non-global line ending regex
zertosh May 15, 2017
484da18
Fix: Use unicode literals instead of escapes in showInvisibles
zertosh May 15, 2017
f58b054
Fix: Use documented range form when calling insertTextAfterRange
zertosh May 15, 2017
a0c8a0e
Fix: Un-shebang test file
zertosh May 15, 2017
7ba59f6
Fix: ESLint 4.x compat (handle shebangs and no deprecated APIs)
zertosh May 15, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Chore: Use eslint-plugin-prettier without symlink
  • Loading branch information
@zertosh
zertosh committed May 14, 2017
commit 3e983f1408dd1e0f88aca8ca48101bbaf85852ed
14 changes: 4 additions & 10 deletions .eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,11 @@
'use strict';

const fs = require('fs');
const path = require('path');
const PACKAGE_NAME = require('./package').name;
const SYMLINK_LOCATION = path.join(__dirname, 'node_modules', PACKAGE_NAME);

// Symlink node_modules/{package name} to this directory so that ESLint resolves this plugin name correctly.
if (!fs.existsSync(SYMLINK_LOCATION)) {
fs.symlinkSync(__dirname, SYMLINK_LOCATION);
}
// Register ourselves as a plugin to avoid `node_modules` trickery.
const Plugins = require('eslint/lib/config/plugins');
Plugins.define('prettier', require('.'));
Copy link
Collaborator

@not-an-aardvark not-an-aardvark May 15, 2017

Choose a reason for hiding this comment

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

I think this approach is going to break after eslint/eslint#8465 is merged, which is likely to happen in ESLint 4. (The change makes Plugins an ES6 class rather than a global mutable object, to prevent unrelated modules from conflicting with each other if they both happen to be using eslint in the same process.)

Copy link
Member Author

@zertosh zertosh May 15, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Oh thanks for the heads up! Modifying the node_modules after an install causes yarn issues :/

QQ: We also use this approach at fb because you can't use a path as a plugin. I understand the module resolution complications of supporting relative paths, but would the ESLint team be open to accepting a PR for absolute paths to a plugin?

Copy link
Collaborator

@not-an-aardvark not-an-aardvark May 15, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

There's an accepted issue for it at eslint/eslint#6237, but when I looked into it a little while ago it seemed like it could introduce problems even when absolute paths are used. Basically, if I load eslint-plugin-foo with a relative path, and I'm also using a shareable config that references something else called eslint-plugin-foo in node_modules (e.g. another version of the plugin), it wouldn't be possible to configure rules from both plugins independently.

Feel free to comment on that thread if you have any ideas for how to handle that, though.

Hmm, I hadn't realized modifying node_modules breaks yarn. Maybe we should keep Plugins somehow until we have a better way to refer to plugins by path.

Copy link
Member Author

@zertosh zertosh May 15, 2017

Choose a reason for hiding this comment

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

Hmm, I hadn't realized modifying node_modules breaks yarn

It causes yarn check to fail. Also, it'll break a new yarn feature I'll be upstreaming soon: "watchman-based check". Basically, you get instant (sub ~100ms) full node_modules verification, which lets use put yarn in front of every js script w/o having to worry about stale modules.


module.exports = {
plugins: ['node', 'eslint-plugin', PACKAGE_NAME],
plugins: ['node', 'eslint-plugin', 'prettier'],
extends: [
'not-an-aardvark/node',
'plugin:node/recommended',
Expand Down