-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Use a type-safe way is differentiating ExecutionResult from Array<Error> #3649
Conversation
…Array.isArray instead.
|
✅ Deploy Preview for compassionate-pike-271cb3 ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Checks failing because lockfile does not match, our ci requires you to update this yourself and then we check to make sure it matches. Putting together your typescript version changes and your proposed code change, it sounds like you are saying that on newest version of typescript, we get a warning on our explicit property check? It could be the simplest thing is to just disable the warning for that line. |
Could be also that this is not secondary to the version change and just a different tsconfig in your project? I skimmed the TS 4.7 release notes and didn't note anything at first glance. |
This comment has been minimized.
This comment has been minimized.
@saihaj Something went wrong, please check log. |
As part of graphql#3649 research I added all options explicitly and also enable ones that didn't require any code change. We should enable rest of them in a separate PRs.
As part of #3649 research I added all options explicitly and also enable ones that didn't require any code change. We should enable rest of them in a separate PRs.
Superseded by #3670 |
Motivation
Checking a string key on an array is not valid, correctly yielding a warning on a TS branch I'm playing in.
Narrow the type with Array.isArray instead.
Performance
Running
yarn benchmark
(which is amazing, btw) on my 2019 MacBook Pro running macOS 12.4 and node 18.2.0, I see a few slight increases and slight decreases of my local vs HEAD. I exited all other user processes on the system, but there was still some slight variability from run to run regardless.Here's a screenshot of the run, let me know if I need to dive into CPU profiler data to find another type-safe alternative that's cheaper at runtime.