bug(useStepper): using object steps relies on object key order being maintained

darrylnoakes commented 2 years ago

Describe the bug

Using objects for the steps relies on the key order of the wrapper object being maintained.

While there are possibilities that the order is entirely random, in general engines have followed certain rules rules for quite a while, with the spec being updated afterward. The order for non-inherited properties is:

Integer-like keys in ascending order
String keys in insertion order
Symbols in insertion order

Expand for more detail on specs and such

There was almost no spec for this prior to ES2015, and it was only properly specced in ES2020. From [an SO answer](https://stackoverflow.com/a/5525820/15261914): "The iteration order for objects follows [a certain set of rules](https://stackoverflow.com/a/38218582/292500) since ES2015, but it does not (always) follow the insertion order." From that second link: > The order for "own" (non-inherited) properties is: > > - Integer-like keys in ascending order > - String keys in insertion order > - Symbols in insertion order > > Thus, there are three segments, which may alter the insertion order (as happened in the example). And integer-like keys don't stick to the insertion order at all. > > In ES2015, only certain methods followed the order: > > - `Object.assign` > - `Object.defineProperties` > - `Object.getOwnPropertyNames` > - `Object.getOwnPropertySymbols` > - `Reflect.ownKeys` > - `JSON.parse` > - `JSON.stringify` > > As of ES2020, all others do (some in specs between ES2015 and ES2020, others in ES2020), which includes: > > - `Object.keys, Object.entries, Object.values, ...` > - `for..in`

While there is the possibility that this doesn't work in older browsers, I don't believe this is an issue, especially moving forward.

However, if this composable is used with a combination of integer keys and non-integer keys, the order will not be maintained, as stated in the rules given above, which is an actual issue. E.g. "Intro, 1, 2, 3, End" would be changed to "1, 2, 3, Intro, End".

~~A potential fix is to rather pass an array of objects, with each an extra property in each object to specify the name.~~ Rejected, as it requires a special property in each step object.

A potential solution is to use the entries structure (ala Object.entries()), where the value is an array instead of an object and each property is given as a tuple of the key and object.
For example:

const stepper = useStepper([
  ['user-information', {
    title: 'User information',
    isValid: () => form.firstName && form.lastName,
  }],
  ['billing-address', {
    title: 'Billing address',
    isValid: () => !!form.billingAddress,
  }],
  ['terms', {
    title: 'Terms',
    isValid: () => form.contractAccepted === true,
  }],
  ['payment', {
    title: 'Payment',
    isValid: () => ['credit-card', 'paypal'].includes(form.payment),
  }],
])

This is not that different in style to the original: the top-level object is changed to an array, and for each item the key-value pair is changed to an array.

It can vene be generated using the original object structure by simply passing the original value to Object.entries().

If possible, a potentially better solution is to allow both argument structures and do that conversion internally: it accepts an object as before, and the entries structure, but internally uses the entries structure and converts an object argument to it using Object.entries().
The drawbacks I see are some implementation complexity and the possibility for fragmentation.

Reproduction

VueUse useStepper key-order issue repro on StackBlitz

System Info

(Taken from the StackBlitz repro.)

System:
  OS: Linux 5.0 undefined
  CPU: (4) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
  Memory: 0 Bytes / 0 Bytes
  Shell: 1.0 - /bin/jsh
Binaries:
  Node: 16.14.2 - /usr/local/bin/node
  Yarn: 1.22.10 - /bin/yarn
  npm: 7.17.0 - /bin/npm
npmPackages:
  @vueuse/core: ^8.9.0 => 8.9.0 
  vue: ^3.2.25 => 3.2.31

Used Package Manager

npm

Validations

[X] Follow our Code of Conduct
[X] Read the Contributing Guidelines.
[X] Read the docs.
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[X] Make sure this is a VueUse issue and not a framework-specific issue. For example, if it's a Vue SFC related bug, it should likely be reported to https://github.com/vuejs/core instead.
[X] Check that this is a concrete bug. For Q&A open a GitHub Discussion.
[X] The provided reproduction is a minimal reproducible example of the bug.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

lbineau commented 1 year ago

I agree this is an issue. Maybe we could we use Map() to ensure the order or an order field inside the object?

darrylnoakes commented 1 year ago

I personally like the method I suggested because of its simplicity and wide compatibility. Maps do not serialize well, and maintaining the order separately would be confusing and a pain. If you do not have the issue of numeric keys, you could use Object.entries() to construct it from an object in the original format.

Hence, I propose it be an overload: it accepts an object as before, and the entries structure, but internally uses the entries structure and converts an object argument to it using Object.entries().

vueuse / vueuse