Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EStyleSheet.subscribe callback get called only one time #109

Closed
dhl1402 opened this issue Jun 14, 2019 · 7 comments
Closed

EStyleSheet.subscribe callback get called only one time #109

dhl1402 opened this issue Jun 14, 2019 · 7 comments

Comments

@dhl1402
Copy link

dhl1402 commented Jun 14, 2019

When style is already built, EStyleSheet.subscribe's callback will get called only one time immediately. Is this designated behavior??

@vitalets
Copy link
Owner

hi @dhl1402 !
No, the callback should be called on every EStyleSheet.build().
Could you show a demo code?

@dhl1402
Copy link
Author

dhl1402 commented Jun 25, 2019

Sorry for late reply. Totally forgot about this.
Here the code of your subscribe function:

subscribe(event, listener) {
    if (event !== BUILD_EVENT) {
      throw new Error(`Only '${BUILD_EVENT}' event is currently supported.`);
    }
    if (typeof listener !== 'function') {
      throw new Error('Listener should be a function.');
    }
    if (this.builded) {
      listener();
    } else {
      this.listeners[BUILD_EVENT] = this.listeners[BUILD_EVENT] || [];
      this.listeners[BUILD_EVENT].push(listener);
    }
  }

If styles is already built, the listener will be called but will not be saved for the next EStyleSheet.build() calls.
I will add a snack later 😀

@dhl1402
Copy link
Author

dhl1402 commented Jun 25, 2019

hi @vitalets, here the reproduce snack. please take a look 😀
https://snack.expo.io/@dhl1402/react-native-extended-stylesheet-reproduce

@vitalets
Copy link
Owner

I've got you point. Yes, this is a bug from the time when there was no re-build. Thanks!
Listener should be saved anyway and called if styles already built:

subscribe(event, listener) {
    if (event !== BUILD_EVENT) {
      throw new Error(`Only '${BUILD_EVENT}' event is currently supported.`);
    }
    if (typeof listener !== 'function') {
      throw new Error('Listener should be a function.');
    }
    this.listeners[BUILD_EVENT] = this.listeners[BUILD_EVENT] || [];
    this.listeners[BUILD_EVENT].push(listener);
    if (this.builded) {
      listener();
    }
  }

@dhl1402
Copy link
Author

dhl1402 commented Jun 27, 2019

Great! I think we need a way to remove the listeners. Maybe return an unsubscribe function or return the index of listener and expose an unsubscribe function

@vitalets
Copy link
Owner

I think we can introduce unsubscribe function:

unsubscribe(event, listener) { }

By the way could you describe your workflow with subscription?
Originally it was done for defining components on the first tick. But it seems to me there are other use-cases.

@dhl1402
Copy link
Author

dhl1402 commented Jun 27, 2019

I don't use subscription for defining components but for re-calculating styles after theme change as mentioned in #47

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants