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

Deprecate passing classBinding and classNameBindings as arguments #691

Merged
merged 4 commits into from
Jan 15, 2021

Conversation

pzuraq
Copy link
Contributor

@pzuraq pzuraq commented Dec 23, 2020


## How We Teach This

### Deprecation Guide
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there an until for this deprecation?

Copy link
Member

Choose a reason for hiding this comment

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

It will be until 4.0

@chancancode chancancode added this to the 4.0 milestone Jan 6, 2021
Comment on lines 139 to 143
Note that we are passing in the `@class` argument, not the `class` attribute.
This is to maintain the existing behavior exactly, but in most cases it will be
safe to convert to using the `class` attribute as well. It is only unsafe if you
were directly referencing the value of the `@class` argument anywhere in your
component.
Copy link
Member

Choose a reason for hiding this comment

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

I suggest to flip this around and use the class= syntax in the example (which is what most people are going to look at and copy) and only mention the caveat in text. I think it's extremely unlikely that this will matter to real apps – you would have to know that classBinding and classNameBindings transforms into the class argument, plus it doesn't give you access to other classes added by the JS code. You could argue this is not really supported even.

@chancancode
Copy link
Member

We discussed this on the framework core team meeting today and decided to move this into the final comment period (assuming @pzuraq is going to apply my suggestion above – but it's a small enough detail that doesn't materially changes much about the RFC).

@rwjblue rwjblue merged commit 94cb144 into master Jan 15, 2021
@rwjblue rwjblue deleted the deprecate-legacy-class-binding-curly-invocation branch January 15, 2021 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants