The READY event is not triggered when the bootstrap is not empty and bootstrapOverride = false.

AsliTuzcuogluBytro commented 2 months ago

Describe the bug

The issue is as follows: The READY event is not triggered when the bootstrap is not empty and bootstrapOverride is set to false.

For example, in the following code snippet, the Ready event will never be triggered. I can use the Init event, but I specifically need the Ready trigger event for other reasons.

My request is for Events.Ready to emit in the example code snippet below:

this.unleash = new UnleashClient({
    url: '...',
    clientKey: '...',
    appName: 'frontend',
    refreshInterval: 3,
    environment: '...',
    context: {
        properties: {
            ....
        },
    },
    bootstrap: [
        {
            enabled: false,
            name: 'test-frontend',
            ...
        },
    ], 
    bootstrapOverride: false,
});

The Ready event is only triggered in two cases.

As you can see, these two code conditions do not cover the case where the bootstrap is not empty and bootstrapOverride is false. (My toggles.length is not equal to 0)

Expected behavior

The READY event needs to be triggered also when the bootstrap is not empty and bootstrapOverride = false.

andreas-unleash commented 2 months ago

Hi @AsliTuzcuogluBytro , thanks for reaching out. For your particular use case, you should listen to the initialized event. Here is the distinction from our docs: initialized - emitted after the SDK has read local cached data in the storageProvider. ready - emitted after the SDK has successfully started and performed the initial fetch towards the Unleash Proxy.

ivarconr commented 2 months ago

@andreas-unleash I think it is a bug though. The SDK should still emit the ready after it has successfully synchronized with the Unleash API for the first time.

gastonfournier commented 2 months ago

We are thinking that when you bootstrap you should rely on the initialized event, not the ready event. Bootstrapping shouldn't emit a ready event and that's what we have in our documentation: https://github.com/Unleash/unleash-proxy-client-js?tab=readme-ov-file#available-events

Short term (minor release or patch)

Unfortunately, removing ready event after bootstrapping is considered a major change as it may cause some issues and we want our users to make a conscious decision. In the short term, we think the best is to emit the ready event twice: one when bootstrapped (as of now), and another time after syncing from Unleash. This would be accomplished by my suggestion in this comment

I don't think bootstrapOverride should be used to control the behavior of the ready event as it's intended for a different purpose. By chance it may serve your needs but would not work in all use cases.

Long term (major release)

For the breaking change, we should remove the this.emit(EVENTS.READY); from https://github.com/Unleash/unleash-proxy-client-js/blob/04e21a10aafbdd663f7528f21f7108b1e122d141/src/index.ts#L349-L356

It's not needed as immediately after we emit the INIT event, therefore when bootstrapping you can rely on the INIT event instead of the READY event, and we save the READY event for the cases in which we synchronize with Unleash

AsliTuzcuogluBytro commented 2 months ago

Thank you so much for quick action

ivarconr commented 2 months ago

@AsliTuzcuogluBytro this has been released as part of version 3.4.0-beta.1. Mind testing it and provide feedback?

AsliTuzcuogluBytro commented 2 months ago

@ivarconr , I will try it out and will provide the feedback soon Thank you very much again!

AsliTuzcuogluBytro commented 2 months ago

I have just had the chance to try the'3.4.0-beta.1'version and I would like to confirm that it has fixed the issue! Thank you so much for addressing and resolving the issue so quickly!

Unleash / unleash-proxy-client-js