Rework error collection, error if tslint.json is present #950
Conversation
| assertIndexdts(dirPath); | ||
| ): Promise<{ errors: string[] }> { | ||
| const errors = []; | ||
| const checkExpectedFilesResult = checkExpectedFiles(dirPath, isLatest); |
There was a problem hiding this comment.
This check is now happening per types version, since those folders can also have lint configs... This code may not run if this kind of testing is disabled, e.g. npmChecksOnly mode, which is unfortunate. Was not sure how to clean be able to run on each versioned dir, though.
| const tsconfigErrors = checkTsconfig(dirPath, getCompilerOptions(dirPath)); | ||
| if (tsconfigErrors.length > 0) { | ||
| throw new Error("\n\t* " + tsconfigErrors.join("\n\t* ")); | ||
| errors.push("\n\t* " + tsconfigErrors.join("\n\t* ")); |
There was a problem hiding this comment.
It may soon be worth collecting structured error data and making a nice reporter/formatter, or using a dependency if you know of a good one. Our newlines and indentation are pretty inconsistent.
There was a problem hiding this comment.
Yeah, I saw this and was like "Oh yeah this has some pretty formatting, doesn't it".
From what I can tell, this eventually gets thrown as an error, so as long as it adds a leading newline, it actually turns out okay. But we don't do that in many cases.
I think running everything is fine. Some eslint errors are trivial enough that you’d want to know if your entire module structure is wrong before fixing the eslint stuff. |
This is a noisy change; meant to just send the PR to error if tslint.json is present, but found myself changing a few other things to prevent dtslint from bailing early for simple problems like "npmignore is wrong" or "OTHER_FILES.txt or tslint.json is present".
We'll now not bail out early on errors, e.g.
putting some random garbage in a dts file in a types version dir:
Then with the new error about having a
tslint.json:Perhaps not bailing out early is unwise; before, if there were errors, we would effectively skip ATTW. Maybe I should just make it not run ATTW if there are preceding errors?