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

Bug: Pseudo rules (:after) are incorrectly combined at runtime #231

Closed
Soreine opened this issue Mar 22, 2017 · 9 comments
Closed

Bug: Pseudo rules (:after) are incorrectly combined at runtime #231

Soreine opened this issue Mar 22, 2017 · 9 comments
Labels

Comments

@Soreine
Copy link
Contributor

Soreine commented Mar 22, 2017

I think I have found a bug with how :after rules are combined. When we combine two styles declaring an :after rule, the subsequent call to css() will merge the styles incorrectly.

Minimal example:

const styleSheet = StyleSheet.create({
    greenSquare: {
        ':after': {
            display: 'block',
            content: '""',
            backgroundColor: '#3a3',
            width: 10,
            height: 10
        }
    },

    top: {
        ':after': {
            borderTop: '3px solid red'
        }
    },

    bottom: {
        ':after': {
            borderBottom: '3px solid red'
        }
    }
});

const Test = React.createClass({
    render() {
        return (
            <div>
                <div className={css(styleSheet.greenSquare, styleSheet.top)}>Top</div>
                <div className={css(styleSheet.greenSquare, styleSheet.bottom)}>Bottom</div>
            </div>
        );
    }
});

Renders:

screen shot 2017-03-22 at 17 19 01

As you can see, the second call css(styleSheet.greenSquare, styleSheet.bottom) incorrectly applies the style from stylesheet.top from the previous call.

@Soreine Soreine changed the title Bug: Pseudo rules (:after) are appended at runtime Bug: Pseudo rules (:after) are incorrectly combined at runtime Mar 22, 2017
@lencioni
Copy link
Collaborator

I think I have a fix this in #216. Here's the test I added: https://github.com/Khan/aphrodite/pull/216/files#r107288862

Can you confirm whether this resolves the issue for you?

@Soreine
Copy link
Contributor Author

Soreine commented Mar 23, 2017

Yes, that's exactly what happens, greenSquare's definition is mutated.
Thanks for your awesome looking PR :)

@lazopm
Copy link
Contributor

lazopm commented Apr 10, 2017

@lencioni @xymostech Any updates on this?
This bug is pretty major, we've ran into it several times.

@xymostech
Copy link
Contributor

@lazopm Sorry, I know this is a bug but have been very busy. I'll hopefully get to reviewing and landing @lencioni's changes this week and making a new release.

@elischutze
Copy link

Looking forward to it!

@alexcoady
Copy link

@xymostech Do you have any idea when this might be released? Our large project has a number of bugs as a result of this issue and we're weighing up whether to roll-back, patch all the issues or wait until a new release is available, it'd be good to have an ETA to help our decision-making! Thank you.

@lencioni
Copy link
Collaborator

I just merged #233. This should be fixed in the next release.

@lencioni lencioni added the bug label Apr 27, 2017
@lencioni
Copy link
Collaborator

I just published v1.2.1. Give it a shot!

@Soreine
Copy link
Contributor Author

Soreine commented Apr 29, 2017

Working great ! You rock

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

No branches or pull requests

6 participants