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

garden.selectors broken in v2.0.0 #136

Open
hkjels opened this issue Jun 9, 2017 · 6 comments
Open

garden.selectors broken in v2.0.0 #136

hkjels opened this issue Jun 9, 2017 · 6 comments
Projects

Comments

@hkjels
Copy link

hkjels commented Jun 9, 2017

Whenever selectors/& is used an exception is raised: Assert failed: (vector? tagged-data)

Using the vector-form works [:&:.foo], but as far as I know, that can not be combined with other selector-fns like selectors/+ etc.

@noprompt
Copy link
Owner

Yes. The selectors namespace has not been updated with an implementation of garden.parse/IParse for CSSSelector. That is the reason you are seeing an exception.

@noprompt noprompt added the bug label Jun 16, 2017
@hkjels
Copy link
Author

hkjels commented Jun 18, 2017

Does that mean, that it's not possible to express this selector at the moment? Or is there some other way around it?

[(selector/& (selector/not :Vertically)) {:flex-direction :row}]

@noprompt
Copy link
Owner

@hkjels No but I think we can resolve this problem by defining and implementation of garden.parse/IParse for the garden.selectors.CSSSelector class. That implementation could return a new AST node type of :css.selector/raw which would just be a string.

In selectors.cljs:

(defrecord CSSSelector [selector]
  garden.parse/IParse
  (-parse [this]
    [:css.selector/raw (css-selector this)]))

and in parse.cljc we'd have

(spec/def ::raw-selector
  string?)

(spec/def ::selector
  (spec/or ::simple-selector ::simple-selector
           ::compound-selector ::compound-selector
           ::complex-selector ::complex-selector
           ::raw-selector ::raw-selector)) ;; Support "raw" selectors but only at the top.

We'd then need to add a method for it in normalize.cljc and compiler.cljc.

This isn't a great solution though because it makes the resolution of nested selectors during the normalization process much more complex.

A better alternative would be to parse the result of calling css-selector and return a selector AST that fits in with what currently exists e.g. a :css.selector/complex node, etc. Since there's existing code which already does some lightweight analysis of CSS selector syntax, I think this would be the best direction to go.

@noprompt noprompt changed the title & selector broken in v2.0 garden.selectors broken in v2.0.0 Jun 18, 2017
@noprompt
Copy link
Owner

See #137

@noprompt
Copy link
Owner

@hkjels Apologies for thinking out loud. The game plan will be to implement a parser for the CSS selector syntax and use that in the implementation of garden.parse/IParse for the garden.selector.CSSSelector class.

@hkjels
Copy link
Author

hkjels commented Jun 18, 2017

Hey, don't apologise. I love it! 👍
I don't quite follow you, but I'll look into it and see if I can grasp what your saying here :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
v2.x.x
Awaiting triage
Development

No branches or pull requests

2 participants