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

Add failing test for unnecessary string escaping #81

Merged

Conversation

beauroberts
Copy link
Collaborator

Occurs in nested JSX that contains single quotes

Issue #80

Occurs in nested JSX that contains single quotes

Issue algolia#80
@vvo
Copy link
Contributor

vvo commented May 26, 2017

Nice, @shouze think you can land a hand here? Thanks to both contributors :)

@shouze
Copy link
Contributor

shouze commented May 26, 2017

@vvo yes for sure, thx @beauroberts for the test to reproduce the issue. Can't manage that today. Will find time to look around & fix before monday.

@vvo
Copy link
Contributor

vvo commented May 29, 2017

ping @shouze :D

@shouze
Copy link
Contributor

shouze commented May 29, 2017

@vvo on my way to fix it sorry... I've been dad for the 2nd time in the past days... so was more busy than usual 😉

@Haroenv
Copy link

Haroenv commented May 29, 2017

Congrats @shouze

@vvo
Copy link
Contributor

vvo commented May 29, 2017

I've been dad for the 2nd time in the past days

Nice! Bravo :)

@shouze
Copy link
Contributor

shouze commented May 30, 2017

@beauroberts @vvo @Haroenv all I can say for now - I've checked - is that's not a regression that have been introduced by #79 (string escaping patch).

@shouze
Copy link
Contributor

shouze commented May 30, 2017

@beauroberts @vvo @Haroenv ok I've pushed this PR with a fix that we can discuss: #82.

I guess that's not an innocent replace so would need an exhaustive test coverage to ensure it won't break anything.

@beauroberts
Copy link
Collaborator Author

Apologies for falsely labelling it a regression @shouze, and congratulations on the new addition to your family!

@vvo vvo merged commit 99adc0e into algolia:master Jun 7, 2017
vvo pushed a commit that referenced this pull request Jun 7, 2017
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.

4 participants