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

Fix CLI bug: NaN error found. NaN error already in baseline. #21

Closed
wdoug opened this issue Jun 4, 2024 · 1 comment · Fixed by #23
Closed

Fix CLI bug: NaN error found. NaN error already in baseline. #21

wdoug opened this issue Jun 4, 2024 · 1 comment · Fixed by #23

Comments

@wdoug
Copy link
Contributor

wdoug commented Jun 4, 2024

After the latest change, when running tsc-baseline check the CLI outputs NaN error found. NaN error already in baseline. at the end of each run.

Screen Shot 2024-06-04 at 1 54 54 PM

Here's a demo repo for seeing the issue: https://github.com/wdoug/tsc-baseline-demo

I dug into this a little bit and realized it is a bug with the compilation. This code:

export const getTotalErrorsCount = (errorMap: Map<string, ErrorInfo>): number =>
  [...errorMap.values()].reduce((sum, info) => sum + info.count, 0)

Is being compiled (in the output dist/cli.module.js file) into:

p=function(e){return[].concat(e.values()).reduce(function(e,r){return e+r.count}

This is wrong. It seems like it is assuming the call to values() on the errorMap is returning an array, but actually that returns an iterator.

For example:

const m = new Map();
const result = [].concat(m.values())
// result is: [MapIterator]

Hopefully just updating whatever library is doing the compilation here should fix this.

BrianBasor added a commit to BrianBasor/tsc-baseline that referenced this issue Jun 5, 2024
This should fix the NaN bug reported in
TimMikeladze#21
BrianBasor added a commit to BrianBasor/tsc-baseline that referenced this issue Jun 5, 2024
@BrianBasor
Copy link
Contributor

Thanks for writing this up @wdoug . I was seeing this little bug too.

And while I'm writing, thanks so much for fixing up the baseline logic to ignore the line and column numbers. Excellent move, and sure makes this package more useful for me. Ditto for the nonzero exit code when there are new errors.

I'll put in a PR with the fix for this. It's pretty sweet that @TimMikeladze appreciated your other PRs and got them merged in. Thanks Tim!

TimMikeladze pushed a commit that referenced this issue Jun 8, 2024
* Use Array.from instead of the spread operator

This should fix the bug reported in
#21

* Fix up the pluralization logic for the situation where there are zero errors

The only number to report as a singular is when there is one item.
Zero should be reported as a plural.
Was: `0 error already in baseline.`
Now: `0 errors already in baseline.`

* Minor changes from code review

- Use strict equality
- Specify that these are new errors

Co-authored-by: Will Douglas <willygdouglas@gmail.com>

* Explain why we used Array.from

Co-authored-by: Will Douglas <willygdouglas@gmail.com>

* Use strict equality in all the revised ternaries

---------

Co-authored-by: Will Douglas <willygdouglas@gmail.com>
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 a pull request may close this issue.

2 participants