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

Remove postcss-styled #3872

Merged
merged 1 commit into from Jan 5, 2019
Merged

Remove postcss-styled #3872

merged 1 commit into from Jan 5, 2019

Conversation

gucong3000
Copy link
Member

No description provided.

@gucong3000 gucong3000 force-pushed the postcss-styled branch 2 times, most recently from 0865a53 to f7b026c Compare January 2, 2019 10:33
@jeddy3
Copy link
Member

jeddy3 commented Jan 2, 2019

@gucong3000 I need a little help understanding this PR.

Has postcss-styled been deprecated?

At the moment "styled" is one of the documented options for the syntax option of the Node API. As such, some users might be relying on it and I believe that we'll break their build if we remove it.

(Incidentally, the error message in that last link needs to be updated...)

@gucong3000 gucong3000 force-pushed the postcss-styled branch 3 times, most recently from dd054ce to ec20633 Compare January 3, 2019 02:08
@gucong3000
Copy link
Member Author

I believe that we'll break their build if we remove it.

I updated the code. styled exists as an alias of postcss-jsx

@hudochenkov
Copy link
Member

I think it would make sense to add deprecation notice for styled option. Could be done in another PR.

Also, I think we should rename jsx to css-in-js. "jsx" a little bit confusing, because it's not for JSX only, but for many CSS-in-JS syntaxes. This is also something for another PR. If we agree, then we can deprecate both styled and jsx names, and add css-in-js all in one PR.

@gucong3000
Copy link
Member Author

Should I make any changes to this PR?

@jeddy3
Copy link
Member

jeddy3 commented Jan 3, 2019

If we agree, then we can deprecate both styled and jsx names, and add css-in-js all in one PR.

SGTM.

Should I make any changes to this PR?

I'd be happy to see the suggestion above rolled into this PR. It'd be great if there was just a css-in-js option, and styled and jsx were aliased to it and deprecated. It'd also be good if we doubled checked all mentions of the syntax options in the code and documentations to make sure they are consistently listed as:

"css-in-js"|"html"|"less"|"markdown"|"sass"|"scss"|"sugarss"

Copy link
Member

@hudochenkov hudochenkov left a comment

Choose a reason for hiding this comment

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

Great job, @gucong3000!

I think deprecation notice better to do in another PR to have things more clear.

Copy link
Member

@jeddy3 jeddy3 left a comment

Choose a reason for hiding this comment

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

@gucong3000 Excellent.

I think deprecation notice better to do in another PR to have things more clear.

SGTM.

@jeddy3 jeddy3 merged commit 044c14b into master Jan 5, 2019
@jeddy3 jeddy3 deleted the postcss-styled branch January 5, 2019 14:13
@jeddy3
Copy link
Member

jeddy3 commented Jan 5, 2019

  • Added: css-in-js syntax option that will replace the existing styled and jsx ones (#3872).

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

Successfully merging this pull request may close these issues.

None yet

4 participants