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

fix(formatting): Make the props "key" and "ref" order predictibale #340

Conversation

armandabric
Copy link
Collaborator

...and fix the CI ^^'

@armandabric armandabric added the bug label Jan 5, 2019
@armandabric armandabric self-assigned this Jan 5, 2019
@armandabric
Copy link
Collaborator Author

Close #339

@armandabric armandabric merged commit 3853463 into algolia:master Jan 5, 2019
@armandabric armandabric deleted the fix/prop-name-sorter-key-and-ref-order branch January 5, 2019 11:25
@vvo
Copy link
Contributor

vvo commented Jan 7, 2019

This is an interesting fix, can you teach me a bit more and explain why it fixes the issue?

@armandabric
Copy link
Collaborator Author

armandabric commented Jan 7, 2019

The order function was not stable. It return a different result depending on the order of the original props because we forgot to enforce the order between the key and the ref values:

Before the fix:

console.log(['key', 'ref']).sort(propNameSorter)); // ['key', 'ref'] 
console.log(['ref', 'key']).sort(propNameSorter));  // ['ref', 'key']

After the fix:

console.log(['key', 'ref']).sort(propNameSorter)); // ['key', 'ref']
console.log(['ref', 'key']).sort(propNameSorter));  // ['key', 'ref']

Why this do not trigger before? I'm not sure: maybe some react internal change ?

@vvo
Copy link
Contributor

vvo commented Jan 7, 2019

Thanks!

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

Successfully merging this pull request may close these issues.

None yet

2 participants