optimizely / react-sdk

React SDK for Optimizely Feature Experimentation and Optimizely Full Stack (legacy)
https://docs.developers.optimizely.com/experimentation/v4.0.0-full-stack/docs/javascript-react-sdk
Apache License 2.0
89 stars 35 forks source link

Allow `default` to be set on an OptimizelyVariation that has `variation` also set #86

Open grug opened 3 years ago

grug commented 3 years ago

This would be a nice quality of life improvement to have as we often have experiment code that looks like this:

<OptimizelyExperiment experiment="some_test">
  <OptimizelyVariation variation="original">
    <OriginalComponent />
  </OptimizelyVariation>
  <OptimizelyVariation variation="some_variant">
    <VariantComponent />
  </OptimizelyVariation>
  <OptimizelyVariation default>
    <OriginalComponent />
  </OptimizelyVariation>
</OptimizelyExperiment>

It would be nice to be able to express the above as:

<OptimizelyExperiment experiment="some_test">
  <OptimizelyVariation default variation="original">
    <OriginalComponent />
  </OptimizelyVariation>
  <OptimizelyVariation variation="some_variant">
    <VariantComponent />
  </OptimizelyVariation>
</OptimizelyExperiment>

As it is more concise and just as readable.

Is there a chance of seeing this improvement land in the SDK?

-Dave

Tamara-Barum commented 1 year ago

Internal ticket created: [FSSDK-8653]

alexparish commented 1 year ago

I'm very keen on this change as my company has suffered production incidents due to misunderstandings around how this works. It was assumed that adding a default attribute to an <OptimizelyVariation> component that has a variation would suffice.

Following those incidents, I felt it necessary to write an ESLint plugin to ensure our engineers implement an <OptimizelyVariation> component that has a default attribute but no variation attribute.

If the internal ticket hasn't been progressed, would you welcome a pull request?

mikechu-optimizely commented 11 months ago

I'm working on this Issue and #225 in this sprint.

Always welcome pull requests. This helps all stakeholders (including internal) understand the requirement / POV as well as shorten up the implementation.

mikechu-optimizely commented 11 months ago

I have a dependent branch ready with this bug fix and test coverage.

grug commented 11 months ago

This is great news! Can't wait to see it.