optimizely / fullstack-labs

Apache License 2.0
13 stars 8 forks source link

feat(datafileUrl): Add ability to initialize with fully qualified url #5

Closed asaschachar closed 5 years ago

asaschachar commented 5 years ago

Summary: This PR enables developers to pass in a parameter datafileUrl to the createInstance API of the SDK. Example:

const optimizely = optimizelySDK.createInstance({
  datafileUrl: 'http://cdn.my-app.com/datafile/GaXr9RoDhRcqXJm3ruskRa.json',
})

This is useful for the onboarding tutorial where the datafile url is a special url which is not the same as the production datafiles. We think this feature will be useful for other consumers of the SDK who might want full control over the hosting of their datafiles too.

cc: @zleach

jordangarcia commented 5 years ago

I agree that this makes onboarding a lot nicer. A few things to note though, this js-web-sdk only works in browsers, not node.

Also for onboarding, I imagine we'd want to keep the experience as realistic as possible. Something like supply sdkKey: 'onboarding' makes a bit more sense to me.

const optimizely = optimizelySDK.createInstance({
  sdkKey: 'onboarding'
})

This type of change would require the onboarding datafile to live in the place where other datafiles live. https://cdn.optimizely.com/datafiles/onboarding.json

asaschachar commented 5 years ago

Do you think it would be useful for SDK consumers who want to do their own datafile hosting my-website.com/optimizely/datafile.json? I'm not sure if there are any requests yet for this feature, but I imagine they would because that would add another level of security for when it changes.

asaschachar commented 5 years ago

When I looked last at the onboarding datafile generation, it looked like we generated a unique datafile per user (I imagine because we don't want multiple users to collide when going through onboarding of the same account). Were you suggesting that we use one key onboarding? Or do you think that sdkKey would be unique per user as well (ex: onboarding-<unique_user_id>)?

asaschachar commented 5 years ago

@mjc1283, @jordangarcia and I spoke offline and we chatted about how this PR won't be necessary if the datafile manager of the newest JavaScript SDK will accept a function which returns a URL to fetch the datafile from. Let me know if we are planning to change this interface to accept a function or not.

asaschachar commented 5 years ago

Closing as this PR is now obsolete.