feat: sort feature strategies #4218
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
| ) { | ||
| return strategy1.sortOrder - strategy2.sortOrder; | ||
| } | ||
| return 0; |
There was a problem hiding this comment.
I feel silly saying this but where do items that end up resolving to 0 actually end up in the final list? I'm guessing this is dependent on the JS engine's implementation of the sort? What if we do something like:
strategies.sort((strategy1, strategy2) => {
const sortOrder1 = typeof strategy1.sortOrder === 'number' ? strategy1.sortOrder : -Infinity;
const sortOrder2 = typeof strategy2.sortOrder === 'number' ? strategy2.sortOrder : -Infinity;
return sortOrder1 - sortOrder2;
});
I want to say that's both very explicit and deterministic independent of whatever JS feels like doing
There was a problem hiding this comment.
They way I read it is 0 means don't change the order. I'm afraid of arithmetics with Infinity in JS since we'll get NaN soon
There was a problem hiding this comment.
Yeah I think it's probably okay, from some reading, most modern JS engines should what we expect here. NaN should also behave like a 0 here too.
This is mostly me nitpicking, I think it's probably okay as is
Does this need a flag? This should be effectively the same result for all of our SDKs so long as we don't change the actual API response |
About the changes
Feature strategies will be sorted in client features. This is required for the new variant strategies where the order of strategies matters for the variant that will be selected.
Important files
Discussion points
Client toggles cache is updated on event log change. We need to add re-ordering event for the re-order to trigger changes.
I'm wondering if this change could be behind a flag.