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

autoSpec should never return undefined for {color: {color}} #1400

Closed
tophtucker opened this issue Mar 27, 2023 · 2 comments
Closed

autoSpec should never return undefined for {color: {color}} #1400

tophtucker opened this issue Mar 27, 2023 · 2 comments
Labels
question Further information is needed

Comments

@tophtucker
Copy link
Contributor

Noticed while working on #1385 — this is, I think, the last undefined that autoSpec is returning:

plot/src/marks/auto.js

Lines 195 to 199 in 02731fb

color: {
value: colorValue ?? null,
reduce: colorReduce ?? null,
...(colorColor !== undefined && {color: colorColor})
},

@mbostock mbostock added the question Further information is needed label Mar 27, 2023
@mbostock
Copy link
Member

I’m not following this comment. It looks to me that the code can never return color: {color: undefined} because of the colorColor !== undefined check? So the type is

type AutoColorMaterializedOptions =
  | {value: ChannelValue | null; reduce: ChannelReduce | null}
  | {value: null; reduce: null; color: string};

which seems correct to me. We don’t want to say color: null when there’s a value or reduce because that would mean setting a constant color of none.

@tophtucker
Copy link
Contributor Author

Oh! Sorry, I filed this hastily; I meant that the color property might not be in the returned object, not that it might be present and undefined (color?: string, not color: string | undefined). If that union type seems fine to you, then it’s fine with me. It felt weird that we’re not returning a fixed shape, but I hadn’t thought through what setting color: null would actually do, so, forget it! 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is needed
Projects
None yet
Development

No branches or pull requests

2 participants