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(atomic): fix atomic react rendering function #1909

Merged
merged 3 commits into from
Apr 11, 2022
Merged

fix(atomic): fix atomic react rendering function #1909

merged 3 commits into from
Apr 11, 2022

Conversation

olamothe
Copy link
Member

@olamothe olamothe commented Apr 11, 2022

Couple seems to have went wrong:

  • With the bump to [email protected], I also bumped some cra dependencies. This caused subtle changes to jest setup in headless-react samples, which means we now have to add extra config for Jest
  • setRenderingFunction in AtomicResultList could be undefined if called before initialize. I modified it so that ResultListCommon is always created first, no matter the entry point.

🤞

https://coveord.atlassian.net/browse/KIT-1557

@olamothe olamothe requested a review from a team as a code owner April 11, 2022 16:42
@github-actions
Copy link

Thanks for your contribution @olamothe !
When your pull-request is ready to be merged, check the box below to merge it

  • Merge! :shipit:

@github-actions
Copy link

Pull Request Report

PR Title

✅ Title follows the conventional commit spec.

Bundle Size

File Old (kb) New (kb) Change (%)
case-assist 195.1 195.1 0
search 274 274 0
product-listing 207.8 207.8 0
product-recommendation 191.4 191.4 0
recommendation 190.1 190.1 0

@olamothe
Copy link
Member Author

🤔 I might need to perform a "manual" publish of some packages to fix everything

@olamothe olamothe merged commit 28217bf into master Apr 11, 2022
@olamothe olamothe deleted the KIT-1557 branch April 11, 2022 17:33
@liviarett
Copy link
Contributor

I'm just a bit confused by this, because ResultListCommon needs this.bindings, which only becomes available after initialize, so how can it be done independently?

@olamothe
Copy link
Member Author

I'm just a bit confused by this, because ResultListCommon needs this.bindings, which only becomes available after initialize, so how can it be done independently?

The initialize function is part of Atomic lifecyle (ie: we control when it's called, with the search interface).

Which means it is possible for any code interacting with our components to call any public function on any component. before we internally have a chance to call initialize.

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.

4 participants