unic / estatico

[DEPRECATED] Estático – Static site generator for frontend unicorns
Other
121 stars 18 forks source link

refactoring of variants, switched from array to object with keys #29

Closed swey closed 7 years ago

swey commented 7 years ago

Old scenario: modules: { teaser: teaserData.variants[5] }

New scenario: modules: { teaser: teaserData.variants.inverted }

caillou commented 7 years ago

On the downside if you use an object, the order is not guaranteed.

swey commented 7 years ago

Right, in theory that's the downside. But it should always be handled in the same way when using the same node module versions, shouldn't it?

For us the usage was more important than controlling the order in the module preview - but we also had no problems with the order so far.

backflip commented 7 years ago

Would a mix make sense? Adding a key property and looking for it using teaser: teaserData.variants.find((variant) => variant.key === 'inverted')? Less handy though.

swey commented 7 years ago

I had no problems controlling the order in the module preview, so far it always matched the order of the defined object keys. So there is no real advantage using arrays?

caillou commented 7 years ago

@swey there is no real way to define the order, in which object are iterated over. An array on the other hand is ordered by definition.

It is not only about what the code does, but also about what the code tells you, when you read it.

My (maybe unfunded) fear, is that we will end up with variants that have an order: n attribute.

I much prefer @backflip's approach of adding a key: 'inverted' attribute to the objects. The obvious downside being, that we need to iterate over the array. But I guess that would be acceptable.

orioltf commented 7 years ago

I agree with @backflip and @caillou. Would you have a look at @corinneroelli PR in implementing @backflip idea? Feedback very much appreciated there. If everyone is happy with that, we can merge that one one and close this one.

flavaflo commented 7 years ago

i like this approach much more than PR #37. the structure of variants is more readable in the data.js and the assignment of variants in a module and or page is visually more clear.

i never had to rely on the correct order of the variants.

backflip commented 7 years ago

Preferred over https://github.com/unic/estatico/pull/37 in Slack vote.