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 incorrect usage of hooks after refactor #89

Merged
merged 1 commit into from
Sep 20, 2022

Conversation

jesperzach
Copy link
Contributor

Fixes #85

Seems like it actually works all the way up until 4.0.0-beta.3 (maybe even 4.0.0-beta.47.0 but it doesn't compile so we can't know), so the change that broke single request must have happened somewhere here: v4.0.0-beta.3...v4.0.0-beta.47.1

The most obvious thing that comes to my mind is the change from class component to functional component for AdvertisingSlot. The useEffect hook used is not semantically equivalent to componentDidMount. Perhaps the asynchronous nature of useEffect causes a wrong order in the flow of registering the ad slots before enabling DFP. It might be that useLayoutEffect is more appropriate here.

@jesperzach
Copy link
Contributor Author

@pahund can you prioritize this one, pretty please? 🤞😅

@pahund
Copy link

pahund commented Sep 20, 2022

Hi Jesper, thanks for taking care of this. Looks good to me!

@pahund pahund merged commit 2f499cc into KijijiCA:master Sep 20, 2022
@pahund
Copy link

pahund commented Sep 20, 2022

The change is now available in the latest beta version react-advertising@4.2.0-beta.29. We plan to release the final version 4.2.0 in October.

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.

singleRequest doesn't work in any versions after 3.0.2
2 participants