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

[wip] Stylis CSS Middleware initial commit #125

Closed
wants to merge 1 commit into from

Conversation

giuseppeg
Copy link
Collaborator

No description provided.

@giuseppeg giuseppeg mentioned this pull request Feb 13, 2017
// Strip stylis' namespace.
str = str.substring(prefix.length).trim();

var i = 0
Copy link
Contributor

@thysultan thysultan Feb 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@giuseppeg Everything after this point including :global should already be handled by stylis and the middleware, Is there a reason to iterate through the characters again?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thysultan in your snippet you split selectors by space str.split(' ') which is not reliable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@giuseppeg Apart from [title="hello world"] h2 an attribute case what other case do you think that will break on. I have the following that will make it work in the above case.

if (pos !== -1) {
	var edge;

	if (/\[.*[ ].*\]/g.exec(str) !== null) {
		edge = true;
		str = str.replace(/ (.*(?=\]))/g, '<-!->$1');
	}

	var parts = str.split(' ');

	for (var i = 0; i < parts.length; i++) {
		var part = parts[i];
		pos = part.indexOf(':');

		parts[i] = pos !== -1 ? part.substring(0, pos) + prefix + part.substring(pos) : part + prefix;
	}

	str = parts.join(' ');

	if (edge === true) {
		str = str.replace(/<-!->/g, ' ');
	}

	return str;
}

Copy link
Contributor

@thysultan thysultan Feb 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be better than the split solution i first came up with.

if (ctx === 1.5) {
	// stylis does not prefix `:global()`
	if (str.indexOf(prefix+' ') === 0) {
		// remove prefix
		str = str.substring(prefix.length).trim();

		// single selector
		if (str.indexOf(' ') !== -1) {
			return str.replace(/.+\]|[^ \n\r\t]+/g, function (match) {
				var indexOf = match.indexOf(':');

				// :hover etc...
				if (indexOf !== -1) {
					// global
					if (match.charCodeAt(indexOf+1) === 103) {
						return match.replace(/:global\((.*)\)/g, '$1').trim();
					}
					else {
						return match.substring(0, indexOf) + prefix + match.substring(indexOf);
					}
				}
				else {
					return match + prefix;
				}
			});
		}
		else {
			return str + prefix;
		}
	}
}

Copy link
Collaborator Author

@giuseppeg giuseppeg Feb 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at this stage my version seems more robust (I get less compilation errors) but I am open to suggestions

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, will test it out locally to see what tests fail, might be vendor prefixing and whitespace issues since stylis has gone through a few patches since.

Copy link
Contributor

@thysultan thysultan Feb 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@giuseppeg Tweaked the middleware a bit to be more robust only thing failing now is source-maps generation https://github.com/thysultan/styled-jsx/commits/master last two commits.

Edit: https://github.com/thysultan/styled-jsx/commits/rewrite-css-compiler-v2

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool! do normal and :global selector support nested parenthesis? e.g. :global(div:not(.foo))

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you create a branch and submit a PR (even if work in progress) I can leave some comments on your code

Copy link
Contributor

@thysultan thysultan Feb 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do normal and :global selector support nested parenthesis? e.g. :global(div:not(.foo))

Yes.

h1 :global(div:not(.foo)) {
	color: red;
}

produces...

h1[data-jsx="123"] div:not(.foo) {color: red;}

@giuseppeg
Copy link
Collaborator Author

we went for #134

@giuseppeg giuseppeg closed this Feb 19, 2017
@leo leo deleted the rewrite-css-compiler branch May 12, 2017 21:30
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

Successfully merging this pull request may close these issues.

2 participants