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.

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

Lint-staged integration doesn't stop on failure #353

Closed
k2snowman69 opened this issue Mar 5, 2019 · 7 comments
Closed

Lint-staged integration doesn't stop on failure #353

k2snowman69 opened this issue Mar 5, 2019 · 7 comments
Assignees
Labels
pending release This issue has been resolved but not released

Comments

@k2snowman69
Copy link
Member

Language

All

Sample input source code

A file of which fails to be parsed due to invalid javascript

Expected output

When running git commit -a -m "asdfasdf" that the file which failed to be parsed to stop lint-staged from continuing

Actual output (or error message)

lint-stage just continues and commits the unsorted file

@k2snowman69
Copy link
Member Author

Well it looks like returning -1 and outputting to the console output is definitely not enough to get lint-staged to exit prematurely so time to look into errors.

@k2snowman69
Copy link
Member Author

Yep, looks like we need to throw an exception annoyingly....

@k2snowman69
Copy link
Member Author

This will require a major build update. Luckily, we're aiming for 3.0.0 soon so we'll lump this in there :-)

@k2snowman69
Copy link
Member Author

Investigating what other tools do so we can be consistent.

Setup

I created a file called ./src/language.ts and it contains the following invalid typescript

what would you do if you wanted to fail?

Tslint

Results in no errors along with a return code of 0

D:\Projects\sortier\cli>npm run tslint -- -c tslint.json --fix ./src/language.ts

> @snowcoders/[email protected] tslint D:\Projects\sortier\cli
> tslint "-c" "tslint.json" "--fix" "./src/language.ts"


D:\Projects\sortier\cli>echo Exit Code is %errorlevel%
Exit Code is 0

D:\Projects\sortier\cli>

Prettier

Prettier did throw an error and also returned a return code of 2. This also seems to make lint-stage fail.

D:\Projects\sortier\cli>npm run prettier -- --write ./src/language.ts

> @snowcoders/[email protected] prettier D:\Projects\sortier\cli
> prettier "--write" "./src/language.ts"

src\language.ts
[error] src\language.ts: SyntaxError: ';' expected. (12:6)
[error]   10 | }
[error]   11 |
[error] > 12 | what would you do if you wanted to fail?
[error]      |      ^
[error]   13 |
npm ERR! code ELIFECYCLE
npm ERR! errno 2
npm ERR! @snowcoders/[email protected] prettier: `prettier "--write" "./src/language.ts"`
npm ERR! Exit status 2
npm ERR!
npm ERR! Failed at the @snowcoders/[email protected] prettier script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\Users\RickyPatel\AppData\Roaming\npm-cache\_logs\2019-07-20T17_51_43_467Z-debug.log

D:\Projects\sortier\cli>echo Exit Code is %errorlevel%
Exit Code is 2

@k2snowman69
Copy link
Member Author

k2snowman69 commented Jul 20, 2019

Investigating a bit more, we have two types of errors it seems...

Unsupported files

There are ones where files aren't supported... These make no sense to fire an error on because there's nothing the consumer can do
Current: We log a message but return exit return of 0

D:\Projects\sortier\cli>npm run run -- "./!(node_modules)/**/*"

> @snowcoders/[email protected] run D:\Projects\sortier\cli
> node bin/index.js "./!(node_modules)/**/*"


Sorting coverage/sort-arrow-sprite.png has failed!
If this is an issue with sortier please provide an issue in Github with minimal source code to reproduce the issue
Error: Could not find language support for file - coverage/sort-arrow-sprite.png

Sortier failed because of an internal error

These are internal errors that sortier can't handle but the source code maybe valid
Current: We log a message but return exit return of 0

Sorting dist/reprinter/index.d.ts has failed!
If this is an issue with sortier please provide an issue in Github with minimal source code to reproduce the issue
Error: Unexpected Exception - Node received is null

Sortier failed because file couldn't be compiled

These are scenarios where the file couldn't be sorted because the source is invalid.
Current: We just continue

D:\Projects\sortier\cli>npm run run -- "./src/**/*"

> @snowcoders/[email protected] run D:\Projects\sortier\cli
> node bin/index.js "./src/**/*"


D:\Projects\sortier\cli>

Conclusion

I think the expected behavior should be:

  • Unsupported files - Should log during diagnostic mode, not throw an error and exit code is 0
  • Sortier failed because of an internal error - Should log during normal mode but not throw an error and exit code is 0
  • Sortier failed because file couldn't be compiled - Should log during normal mode and throw an error if possible with exit code !0

That latter one should really be caught by people's builds to be honest but we shouldn't fail silently

@k2snowman69
Copy link
Member Author

Unsupported files should log in diagnostic mode - #579

@k2snowman69 k2snowman69 added the pending release This issue has been resolved but not released label Sep 17, 2019
@k2snowman69
Copy link
Member Author

Released with 3.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending release This issue has been resolved but not released
Projects
None yet
Development

No branches or pull requests

1 participant