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

Update Compiler Fork #8

Closed
thysultan opened this issue Dec 5, 2016 · 24 comments
Closed

Update Compiler Fork #8

thysultan opened this issue Dec 5, 2016 · 24 comments

Comments

@thysultan
Copy link
Contributor

There's a few things missing pre-v0.6.4 of the stylis compiler that your fork will not handle that could be nice to have.

  1. flat css
/* this should get wrapped in the namespace */
color: red;

h1 {
    font-size:1px;
}
  1. full prefix support in @root and @keyframes blocks.

  2. support opting out of name-spacing keyframes and animations.

I've done away with the regex solution i was using, and added supports for stylis("[data-id=namespace]") namespaces.

@rauchg
Copy link
Member

rauchg commented Dec 7, 2016

@thysultan what do you think about :host support instead?

@thysultan
Copy link
Contributor Author

@rauchg So these will be the same& { ... } === :host { ... } ?

@rauchg
Copy link
Member

rauchg commented Dec 7, 2016

yep so

<div>
  <style jsx>{`
    :host {
      color: red
    }
  `}</style>
</div>

:host would impact the div.

Alternatively, we could make the div also targetable simply with div { }, but I'm not sure if that's elegant

@rauchg
Copy link
Member

rauchg commented Dec 7, 2016

cc @giuseppeg

@thysultan
Copy link
Contributor Author

sure i can add that, :host will be an alias for &.

@giuseppeg
Copy link
Collaborator

@rauchg correct :host should resolve to the shadow root aka the div in the example.

One thing to keep in mind is that styles applied from the outside should win over :host's but I guess that this behavior is hard to shim.

Nice to have then it would be to get :host-context(selector) support e.g.:

<body>
  <div>
    <style jsx>{`
      :host-context(body) {
        color: red
      }
    `}</style>
  </div>
</body>

yields

body div[data-jsx="abc"] { color: red }

But probably it is better to tackle this later and maybe also consider the ._aB1bC-ext thing that I suggested in the original issue.

@thysultan
Copy link
Contributor Author

thysultan commented Dec 7, 2016

@rauchg @giuseppeg pushed a patch for all the host variations

:host - > ${namespace} {...}
:host(.fancy), -> ${namspace}.fancy {...}
:host-context(body) -> body ${namespace} {...}

commit

@giuseppeg
Copy link
Collaborator

giuseppeg commented Dec 7, 2016

@thysultan cool in order for * {} to work you need to append the namespace though otherwise ${namespace}* {} won't work. In general I think that suffixing is better than prefixing.

@thysultan
Copy link
Contributor Author

@giuseppeg * {} will generate ${namespace} * {} with a space.

@giuseppeg
Copy link
Collaborator

@thysultan oh I see, sorry I was thinking in terms of styled-jsx :)

@thysultan
Copy link
Contributor Author

thysultan commented Dec 7, 2016

@rauchg instead of this

  <style jsx>{`
    :host {
      color: red
    }
  `}</style>

can this work

<style jsx>`
    :host {
      color: red
    }
`</style>

or better yet a plugin that treats everything in the <style jsx><style> block as a text node/string literal, removing the need for \`or{}`

<style jsx>
    :host {
      color: red
    }
</style>

@rauchg
Copy link
Member

rauchg commented Dec 7, 2016

The latter is invalid JSX unfortunately. It'll think of { … } as an expression

@rauchg
Copy link
Member

rauchg commented Dec 7, 2016

We picked the most optimal syntax considering how JSX works today I think :D

@rauchg
Copy link
Member

rauchg commented Dec 16, 2016

@thysultan what happens here

https://github.com/zeit/styled-jsx/blob/master/lib/style-transform.js#L197

if the selector is something like:

[title="a,b"] {
  color: red
}

@thysultan
Copy link
Contributor Author

@rauchg it will break, i fixed this in 0.7.1 commit

@giuseppeg
Copy link
Collaborator

giuseppeg commented Feb 10, 2017

@thysultan do you think that it would be trivial to switch to your version of the compiler and write a middleware to add scoping the styled-jsx way (with data-jsx to each selector)? If so would you be interested in helping out?

@thysultan
Copy link
Contributor Author

thysultan commented Feb 10, 2017

Sure, the middleware will look something like this

function middleware (ctx, str, line, col, prefix) {
	if (ctx === 1.5) {
                // avoids name-spacing :global() functions
		if (str.indexOf(prefix) === 0) {
			return str.replace(prefix, '') + prefix;
		}
	}
}

This will change h1, h2 { to h1[prefix], h2[prefix] {

The 1.5 context is to be executed at every selector(post-processed) in a selector declaration.

@thysultan
Copy link
Contributor Author

@giuseppeg On the repl site https://stylis.js.org/ You can try the following in the console.

stylis.use(middleware (ctx, str, line, col, prefix) {
	if (ctx === 1.5) {
		if (str.indexOf(prefix+' ') === 0) {
			var pos = (str = str.substring(prefix.length).replace('&', '').trim()).indexOf(' ');
			return pos !== -1 ? str.substring(0, pos) + prefix + str.substring(pos) : str + prefix;
		}
	}
});

and input something into the editor and it should namespace the styled-jsx way.

@giuseppeg
Copy link
Collaborator

@thysultan the selector below should be .logo[data-jsx="foo"] a[data-jsx="foo"]

screen shot 2017-02-10 at 5 06 56 pm

i.e. the prefix should be added to every part of the selector (except for globals). Do you think that it is doable with the current version of stylis + middleware?

@thysultan
Copy link
Contributor Author

@giuseppeg Yes doable, what should .logo:hover a { compile to?

@giuseppeg
Copy link
Collaborator

cool .logo[data-jsx="foo"]:hover a[data-jsx="foo"]

@thysultan
Copy link
Contributor Author

thysultan commented Feb 10, 2017

Then this should work as expected.

function middleware (ctx, str, line, col, prefix) {
	if (ctx === 1.5) {
                // avoid :global()
		if (str.indexOf(prefix+' ') === 0) {
                        // strips stylis added namespace
			str = str.substring(prefix.length).trim();
                        // flag if single selector
			var pos = str.indexOf(' ');
                        // multiple selectors, i.e h1 h2
			if (pos !== -1) {
				var parts = str.split(' ');
                                // add namespace to all parts
				for (var i = 0; i < parts.length; i++) {
					var part = parts[i];
					pos = part.indexOf(':');
                                        // if has pseudo selector
					parts[i] = pos !== -1 ? part.substring(0, pos) + prefix + part.substring(pos) : part + prefix;
				}

				return parts.join(' ');
			}
                        // single selector
			else {
				return str + prefix;
			}
		}
	}
}

@giuseppeg
Copy link
Collaborator

@thysultan @rauchg I started to work on this tonight #125 keep in mind that it is very wip

@giuseppeg
Copy link
Collaborator

fixed by #134

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

No branches or pull requests

3 participants