fix(config): Use named bin in lintstagedrc.js #3

sudo-suhas · 2017-09-19T04:22:42Z

What:
Fix a bug on windows related to lint-staged config.

Why:
On windows, this happens when trying to run lint-staged:

λ git commit
husky > npm run -s precommit (node v8.5.0)

25l[08:47:32] Running tasks for **/*.+(js|json|less|css|ts) [started]
[08:47:32] Running tasks for .all-contributorsrc [started]
[08:47:32] Running tasks for .all-contributorsrc [skipped]
[08:47:32] → No staged files match .all-contributorsrc
[08:47:32] C:\Program Files\nodejs\node.exe E:\Projects\repos\glamorous\node_modules\kcd-scripts\dist\index.js format [started]
[08:47:32] C:\Program Files\nodejs\node.exe E:\Projects\repos\glamorous\node_modules\kcd-scripts\dist\index.js format [failed]
[08:47:32] → C:\Program could not be found. Try `npm install C:\Program`.
[08:47:32] Running tasks for **/*.+(js|json|less|css|ts) [failed]
[08:47:32] → C:\Program could not be found. Try `npm install C:\Program`.
25hC:\Program could not be found. Try `npm install C:\Program`.
25h
husky > pre-commit hook failed (add --no-verify to bypass)

How:
In the lintstagedrc.js, instead of using absolute paths to node binary and kcd-scripts binary, used the named bin itself. lint-staged is smart enough to figure out the rest.

I added an entry in package.json>scripts required for lint-staged to work in this repo. I also added a script for formatting. Hope that's alright.

Checklist:

Documentation
Tests
Ready to be merged
Added myself to contributors table

To be clear, how the named binary works, is different in this repo and others. In this repo, in order for lint-staged to know what kcd-scripts means, the scripts entry in package.json helps it resolve it to dist/index.js. In all other repos, lint-staged looks and resolves kcd-scripts by peeking into node_modules/.bin.

Closes #2.

codecov · 2017-09-19T04:24:59Z

Codecov Report

Merging #3 into master will not change coverage.
The diff coverage is n/a.

@@         Coverage Diff          @@
##           master    #3   +/-   ##
====================================
  Coverage       0%    0%           
====================================
  Files          10    10           
  Lines         130   128    -2     
  Branches       25    25           
====================================
+ Misses        105   103    -2     
  Partials       25    25

Impacted Files	Coverage Δ
src/config/lintstagedrc.js	`0% <ø> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8a34fa5...851a370. Read the comment docs.

kentcdodds

Hey thanks! Seems to work great. Just had a quick question :)

kentcdodds · 2017-09-20T03:15:26Z

package.json

    "validate": "node src validate",
-    "precommit": "node src precommit"
+    "precommit": "node src precommit",
+    "kcd-scripts": "dist/index.js"


What is the purpose of this?

In this repo, in order for lint-staged to know what kcd-scripts means, the scripts entry in package.json helps it resolve it to dist/index.js.

Here's the relevant documentation section from lint-staged - https://github.com/okonet/lint-staged#what-commands-are-supported

The other mind-bending option would be to add kcd-scripts as a dependency of kcd-scripts 😆.

Oh, I see... In that case, I'd really prefer to solve this another way... I don't want to force folks to add a script called kcd-scripts to make the precommit hook work... I think that if we actually just add quotes around the executor (via https://www.npmjs.com/package/any-shell-escape maybe?) that would solve it.

edit: collapsed comment

No @kentcdodds That is required only for this package. All other packages who depend on kcd-scripts have a bin file in ./node_modules/.bin/kcd-scripts:

E:\Projects\repos\glamorous (master) (glamorous@0.0.0-semantically-released) λ cat .\node_modules\.bin\kcd-scripts #!/bin/sh basedir=$(dirname "$(echo "$0" | sed -e 's,\\,/,g')") case `uname` in *CYGWIN*) basedir=`cygpath -w "$basedir"`;; esac if [ -x "$basedir/node" ]; then "$basedir/node" "$basedir/../kcd-scripts/dist/index.js" "$@" ret=$? else node "$basedir/../kcd-scripts/dist/index.js" "$@" ret=$? fi exit $ret

And lint-staged uses npm-which to resolve kcd-scripts to this. There is no need for kcd-scritps to make any changes. I already tested this in glamorous:

cat .\node_modules\kcd-scripts\dist\config\lintstagedrc.js:

'use strict'; var _slicedToArray = function () { function sliceIterator(arr, i) { var _arr = []; var _n = true; var _d = false; var _e = undefined; try { for (var _i = arr[Symbol.iterator](), _s; !(_n = (_s = _i.next()).done); _n = true) { _arr.push(_s.value); if (i && _arr.length === i) break; } } catch (err) { _d = true; _e = err; } finally { try { if (!_n && _i["return"]) _i["return"](); } finally { if (_d) throw _e; } } return _arr; } return function (arr, i) { if (Array.isArray(arr)) { return arr; } else if (Symbol.iterator in Object(arr)) { return sliceIterator(arr, i); } else { throw new TypeError("Invalid attempt to destructure non-iterable instance"); } }; }(); var _process$argv = _slicedToArray(process.argv, 1), executor = _process$argv[0]; // Only changed the following line var scripts = `kcd-scripts`; module.exports = { linters: { '**/*.+(js|json|less|css|ts)': [`${scripts} format`, `${scripts} lint`, `${scripts} test --findRelatedTests`, 'git add'], '.all-contributorsrc': [ // lint-staged passes arguments to the scripts. // to avoid passing these arguments, we do the echo thing `${scripts} contributors generate`, 'git add README.md'] } };

E:\Projects\repos\glamorous (master) (glamorous@0.0.0-semantically-released) λ git status On branch master Your branch is behind 'origin/master' by 1 commit, and can be fast-forwarded. (use "git pull" to update your local branch) Changes to be committed: (use "git reset HEAD <file>..." to unstage) modified: src/cjs-entry.js E:\Projects\repos\glamorous (master) (glamorous@0.0.0-semantically-released) λ git commit husky > npm run -s precommit (node v8.5.0) 25l[08:56:55] Running tasks for **/*.+(js|json|less|css|ts) [started] [08:56:55] Running tasks for .all-contributorsrc [started] [08:56:55] Running tasks for .all-contributorsrc [skipped] [08:56:55] → No staged files match .all-contributorsrc [08:56:55] kcd-scripts format [started] [08:56:56] kcd-scripts format [completed] [08:56:56] kcd-scripts lint [started] [08:57:02] kcd-scripts lint [failed] [08:57:02] → × kcd-scripts lint found some errors. Please fix them and try committing again. E:\Projects\repos\glamorous\src\cjs-entry.js 4:7 error 'abc' is assigned a value but never used. Allowed unused vars must match /^ignored/ no-unused-vars ✖ 1 problem (1 error, 0 warnings) [08:57:02] Running tasks for **/*.+(js|json|less|css|ts) [failed] [08:57:02] → × kcd-scripts lint found some errors. Please fix them and try committing again. E:\Projects\repos\glamorous\src\cjs-entry.js 4:7 error 'abc' is assigned a value but never used. Allowed unused vars must match /^ignored/ no-unused-vars ✖ 1 problem (1 error, 0 warnings) 25h× kcd-scripts lint found some errors. Please fix them and try committing again. E:\Projects\repos\glamorous\src\cjs-entry.js 4:7 error 'abc' is assigned a value but never used. Allowed unused vars must match /^ignored/ no-unused-vars ✖ 1 problem (1 error, 0 warnings) 25h husky > pre-commit hook failed (add --no-verify to bypass)

Symlink is pretty hit-or-miss on windows. I tested this and seems to be fine on Windows 10. However, the changes you had done for eslint have introduced errors.

eslint errors

λ yarn run lint yarn run v1.0.2 $ node src lint E:\Projects\repos\kcd-scripts\other\symlink.js 4:7 error Expected a function declaration func-style E:\Projects\repos\kcd-scripts\src\config\babelrc.js 13:4 error '?' should be placed at the end of the line operator-linebreak 14:4 error ':' should be placed at the end of the line operator-linebreak 28:8 error '?' should be placed at the end of the line operator-linebreak 29:1 error Expected indentation of 8 spaces but found 10 indent 30:1 error Expected indentation of 8 spaces but found 10 indent 31:1 error Expected indentation of 6 spaces but found 8 indent 32:8 error ':' should be placed at the end of the line operator-linebreak 34:8 error '?' should be placed at the end of the line operator-linebreak 35:8 error ':' should be placed at the end of the line operator-linebreak 37:8 error '?' should be placed at the end of the line operator-linebreak 38:1 error Expected indentation of 8 spaces but found 10 indent 39:1 error Expected indentation of 8 spaces but found 10 indent 40:1 error Expected indentation of 6 spaces but found 8 indent 41:8 error ':' should be placed at the end of the line operator-linebreak 43:8 error '?' should be placed at the end of the line operator-linebreak 44:8 error ':' should be placed at the end of the line operator-linebreak E:\Projects\repos\kcd-scripts\src\config\jest.config.js 4:7 error Expected a function declaration func-style E:\Projects\repos\kcd-scripts\src\config\rollup.config.js 12:7 error Expected a function declaration func-style 13:7 error Expected a function declaration func-style E:\Projects\repos\kcd-scripts\src\index.js 13:18 error 'ignoredBin' is assigned a value but never used no-unused-vars E:\Projects\repos\kcd-scripts\src\scripts\build\babel.js 7:7 error Expected a function declaration func-style 12:4 error '?' should be placed at the end of the line operator-linebreak 13:4 error ':' should be placed at the end of the line operator-linebreak 16:4 error '?' should be placed at the end of the line operator-linebreak 17:4 error ':' should be placed at the end of the line operator-linebreak E:\Projects\repos\kcd-scripts\src\scripts\build\rollup.js 18:7 error Expected a function declaration func-style 24:4 error '?' should be placed at the end of the line operator-linebreak 25:4 error ':' should be placed at the end of the line operator-linebreak 28:4 error '?' should be placed at the end of the line operator-linebreak 29:4 error ':' should be placed at the end of the line operator-linebreak 40:7 error Expected a function declaration func-style 47:4 error '?' should be placed at the end of the line operator-linebreak 48:4 error ':' should be placed at the end of the line operator-linebreak E:\Projects\repos\kcd-scripts\src\scripts\format.js 9:7 error Expected a function declaration func-style 16:4 error '?' should be placed at the end of the line operator-linebreak 17:4 error ':' should be placed at the end of the line operator-linebreak 22:4 error '?' should be placed at the end of the line operator-linebreak 23:4 error ':' should be placed at the end of the line operator-linebreak E:\Projects\repos\kcd-scripts\src\scripts\lint.js 7:7 error Expected a function declaration func-style 15:4 error '?' should be placed at the end of the line operator-linebreak 16:4 error ':' should be placed at the end of the line operator-linebreak 24:4 error '?' should be placed at the end of the line operator-linebreak 25:4 error ':' should be placed at the end of the line operator-linebreak E:\Projects\repos\kcd-scripts\src\scripts\test.js 15:6 error '?' should be placed at the end of the line operator-linebreak 16:6 error ':' should be placed at the end of the line operator-linebreak 24:4 error '?' should be placed at the end of the line operator-linebreak 25:4 error ':' should be placed at the end of the line operator-linebreak E:\Projects\repos\kcd-scripts\src\scripts\validate.js 19:4 error '?' should be placed at the end of the line operator-linebreak 20:1 error Expected indentation of 4 spaces but found 6 indent 21:1 error Expected indentation of 4 spaces but found 6 indent 22:1 error Expected indentation of 4 spaces but found 6 indent 23:1 error Expected indentation of 4 spaces but found 6 indent 24:1 error Expected indentation of 2 spaces but found 4 indent 25:1 error Expected indentation of 4 spaces but found 6 indent 26:1 error Expected indentation of 6 spaces but found 8 indent 27:1 error Expected indentation of 4 spaces but found 6 indent 28:1 error Expected indentation of 4 spaces but found 6 indent 29:1 error Expected indentation of 2 spaces but found 4 indent 30:4 error ':' should be placed at the end of the line operator-linebreak 31:1 error Expected indentation of 4 spaces but found 6 indent 32:1 error Expected indentation of 4 spaces but found 6 indent 33:1 error Expected indentation of 2 spaces but found 4 indent E:\Projects\repos\kcd-scripts\src\utils.js 22:7 error Expected a function declaration func-style 23:7 error Expected a function declaration func-style 24:7 error Expected a function declaration func-style 24:16 error Arrow function used ambiguously with a conditional expression no-confusing-arrow 27:7 error Expected a function declaration func-style 29:7 error Expected a function declaration func-style 32:7 error Expected a function declaration func-style 32:33 error Arrow function used ambiguously with a conditional expression no-confusing-arrow 39:7 error Expected a function declaration func-style 45:7 error Expected a function declaration func-style ✖ 73 problems (73 errors, 0 warnings) 55 errors, 0 warnings potentially fixable with the `--fix` option. error Command failed with exit code 1. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Weird 🤔 I'm not seeing similar errors. Make sure you have latest?

Had to run yarn, that fixed it.

I fixed it so we don't need symlinks :) ec4164e

Also relevant: 11e690a

sudo-suhas · 2017-09-21T03:22:47Z

@kentcdodds I have to apologise. The PR I submitted was faulty. I had added kcd-scripts as a dependency while I was testing and that was the reason it was working for me. The problem is that lint-staged actually looks for the package.json>scripts entry for kcd-scripts format instead of kcd-scripts. I have created an issue in lint-staged for this - lint-staged/lint-staged#294.

kentcdodds · 2017-09-21T03:25:52Z

No worries @sudo-suhas! Thanks for digging into things! :)

sudo-suhas · 2017-09-22T08:54:43Z

@kentcdodds This issue has now been fixed in lint-staged. Would you like me to make a PR removing the duplicated logic between kcd-scripts and lint-staged?

kentcdodds · 2017-09-22T16:26:05Z

Nah, I actually like the new solution better :)

Thanks!

closes kentcdodds#1

sudo-suhas added 2 commits September 19, 2017 09:37

fix(config): Use named bin in lintstagedrc.js

3b5ea16

docs(contributors): Add @sudo-suhas

851a370

kentcdodds reviewed Sep 20, 2017

View reviewed changes

kentcdodds merged commit 78d450c into kentcdodds:master Sep 20, 2017

sudo-suhas deleted the fix/win-lintstagedrc branch September 20, 2017 03:30

layershifter pushed a commit to layershifter/kcd-scripts that referenced this pull request Mar 29, 2021

test(todo): add a bunch of todo tests from cssjanus (kentcdodds#3)

4bee224

closes kentcdodds#1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(config): Use named bin in lintstagedrc.js #3

fix(config): Use named bin in lintstagedrc.js #3

sudo-suhas commented Sep 19, 2017

codecov bot commented Sep 19, 2017 •

edited

Loading

kentcdodds left a comment

kentcdodds Sep 20, 2017

sudo-suhas Sep 20, 2017

sudo-suhas Sep 20, 2017

kentcdodds Sep 20, 2017

sudo-suhas Sep 20, 2017 •

edited

Loading

sudo-suhas Sep 20, 2017

kentcdodds Sep 20, 2017

sudo-suhas Sep 20, 2017

kentcdodds Sep 20, 2017

kentcdodds Sep 20, 2017

sudo-suhas commented Sep 21, 2017

kentcdodds commented Sep 21, 2017

sudo-suhas commented Sep 22, 2017

kentcdodds commented Sep 22, 2017

fix(config): Use named bin in lintstagedrc.js #3

fix(config): Use named bin in lintstagedrc.js #3

Conversation

sudo-suhas commented Sep 19, 2017

codecov bot commented Sep 19, 2017 • edited Loading

Codecov Report

kentcdodds left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sudo-suhas Sep 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sudo-suhas commented Sep 21, 2017

kentcdodds commented Sep 21, 2017

sudo-suhas commented Sep 22, 2017

kentcdodds commented Sep 22, 2017

codecov bot commented Sep 19, 2017 •

edited

Loading

sudo-suhas Sep 20, 2017 •

edited

Loading