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

fix: onContextChanged not running for named providers #491

Merged
merged 5 commits into from
Jul 24, 2023

Conversation

lukas-reining
Copy link
Member

This PR

Fixes context propagation for the static-context-paradigm as found in #488.

Related Issues

Fixes #488

Notes

Follow-up Tasks

How to test

Copy link
Member

@thomaspoignant thomaspoignant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice and simple 👍

@lukas-reining
Copy link
Member Author

lukas-reining commented Jul 21, 2023

The onContextChanged is now called for all providers.
I am not sure if some requirement is still missing for the new spec @toddbaert @beeme1mr? I could not find anything at first sight.

I think what we still have to do is check the numbering of all tests as we mostly used requirement names for the describe blocks, and the number shifted a bit.

@beeme1mr
Copy link
Member

I've created an issue to update the review and update test names to match the updated spec.

#492

Copy link
Member

@toddbaert toddbaert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@toddbaert
Copy link
Member

toddbaert commented Jul 24, 2023

@lukas-reining I'm not sure this is a feat, I think it was more of a fix. I'd recommend fix: onContextChanged not running for named providers, but I'm not 100% sure it matters TBH, we are still sub 1.0/experimental.

Though TBH, we should probably publish our first non-experimental soon.

@beeme1mr beeme1mr changed the title feat: Implement static context paradigm for client sdk fix: onContextChanged not running for named providers Jul 24, 2023
@beeme1mr beeme1mr merged commit 1ab0cc6 into main Jul 24, 2023
@beeme1mr beeme1mr deleted the fix/static-context-propagation branch July 24, 2023 20:40
beeme1mr pushed a commit that referenced this pull request Jul 24, 2023
🤖 I have created a release *beep* *boop*
---


##
[0.3.8-experimental](web-sdk-v0.3.7-experimental...web-sdk-v0.3.8-experimental)
(2023-07-24)


### Features

* typesafe event emitter
([#490](#490))
([92e3a72](92e3a72))


### Bug Fixes

* onContextChanged not running for named providers
([#491](#491))
([1ab0cc6](1ab0cc6))
* race adding handler during init
([#501](#501))
([0be9c5d](0be9c5d))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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

Successfully merging this pull request may close these issues.

web-sdk: onContextChange not called for named provider
4 participants