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

[css-typed-om-1] Duplicating a CSSVariableReferenceValue is harder than it probably should be #665

Closed
bzbarsky opened this issue Feb 15, 2018 · 6 comments

Comments

@bzbarsky
Copy link

Say I have a https://drafts.css-houdini.org/css-typed-om-1/#cssvariablereferencevalue and I want to create a "copy" of it. How do I do that?

new CSSVariableReferenceValue(myValue)

throws.

new CSSVariableReferenceValue(myValue.variable, myValue.fallback)

throws if I have no fallback.

new CSSVariableReferenceValue(myValue.variable, myValue.fallback || undefined)

works, but who would think of doing that? ;)

Ideally there would be a constructor taking CSSVariableReferenceValue.

In addition to that, the second (optional) ctor arg should perhaps be nullable and default to null....

@darrnshn
Copy link
Collaborator

Do we need similar clone methods for other subclasses for consistency?

@bzbarsky
Copy link
Author

I don't know. Do you expect people to clone these things?

This one just jumped out at me because extracting information from a CSSVariableReferenceValue that can be used to construct another CSSVariableReferenceValue was nontrivial.

@darrnshn
Copy link
Collaborator

Yeah CSSVariableReferenceValue is a nontrivial one. I think the transforms also might be hard to clone because of is2D subtleties. That being said, I don't expect people to clone these things, since performant Typed OM code should reuse objects as much as possible.

@tabatkins
Copy link
Member

For now, I at least fixed it so that the fallback argument was nullable, so you can just spam the two fields into the constructor and get something equivalent. Adding constructors for everything that take an existing instance and duplicate it is probably something good to do as well, but I'll defer it for now.

@bzbarsky
Copy link
Author

For now, I at least fixed it so that the fallback argument was nullable,

Why not make null the default value too, then, to make the algorithm simpler?

@annevk annevk reopened this Feb 22, 2018
@tabatkins
Copy link
Member

Yeah, makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants