React v17 support #32

stephensauceda · 2023-02-22T20:06:13Z

What does this PR do?

Bumps dependencies to support React 17. This is package blocks some of our other apps and services from being able to upgrade due to peerDependencies (namely oxygen and the auth migration).

How can this change be manually tested?

I ran the tests with yarn test. It appears two tests are failing. But they appear to be the same two tests that fail on master. Not sure how you guys are rolling with this. Are the tests known failures?

SUMMARY:
✔ 65 tests completed
✖ 2 tests failed

FAILED TESTS:
Bling
✖ reflects outOfPage props to adSlot
Chrome 110.0.0 (Mac OS X 10.15.7)
Error: Uncaught TypeError: Cannot read properties of undefined (reading 'INTERSTITIAL') (webpack:///src/Bling.js:9:23639 <- test/Bling.spec.js:21225)

✖ sets slotSize to 0,0 on foldCheck of 'fluid' or ['fluid']
Chrome 110.0.0 (Mac OS X 10.15.7)
Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.

To test the ad loading stuff:

pull down this branch.
In the react-gpt directory, yarn and yarn build then, run npm link
Open up oxygen in your terminal
Switch to the s/react17 branch
Run npm link @atmedia/react-gpt
Load up the sites as you normally would locally.

Asana Card

https://app.asana.com/0/1201709980437746/1203717210397677/f

Questions or concerns about this change?

When I bump the React dependencies to v17, I get a peerDependency warning for Radium. This is a bummer because it looks like it's a package that is no longer supported by the maintainers. They encourage forks, so... I guess we do that?

Deployment Notes

~~Need to resolve the peerDependency on Radium and React v16~~ Radium has React 17 support in 0.26.2

PR Readiness

Changes have been validated in local dev

Code Readiness

Code follows AT Media styles and conventions
Code has been auto-formatted and is free of lint errors
Unrelated code changes are not present
Relevant inline documentation or READMEs have been updated
Application and browser logs reviewed for runtime warnings and errors
Correct level of logging added to code

Automated Testing

Unit tests are written at the appropriate level given the code’s risk-level
Existing tests still pass
No unnecessary test runner output, e.g. debugging or warnings are not present in Jest or Unittest output

Other Considerations

Relevant documentation updated in Slite

Also consider this PR's impact on architecture and security; AMP, Apple News, RSS; SEO, GA, performance and accessibility.

Please see Post Merge for additional responsibilities after this PR is merged.

stephensauceda · 2023-02-22T20:07:58Z

package.json

@@ -78,10 +85,9 @@
        "prop-types": "^15.5.10",
        "querystring": "^0.2.0",
        "radium": "^0.25.1",
-        "react": "^16.0.0",
-        "react-addons-test-utils": "^15.0.1",


It looks like you guys have already migrated to using react-dom/test-utils so this doesn't appear to be needed anymore

stephensauceda · 2023-02-22T20:08:41Z

package.json

    },
    "scripts": {
        "commit": "git-cz",
        "changelog": "conventional-changelog -p angular -i CHANGELOG.md -s",
        "build": "npm run clean && npm run compile",
        "build:es": "BABEL_ENV=es babel --copy-files ./src -d es",
-        "build:umd":


Prettier got a hold of some of this. The only relevant changes are the deps above.

stephensauceda · 2023-03-01T20:35:36Z

package.json

@@ -1,6 +1,6 @@
 {
    "name": "@atmedia/react-gpt",
-    "version": "2.1.6",
+    "version": "2.1.7",


I just bumped this because 🤷 If we're being semantic, it's probably a major version change.

stephensauceda · 2023-03-01T20:37:15Z

src/Bling.js

            slotSize,
            this.viewableThreshold
        );

        const isHidden = () => {
-            const el = ReactDOM.findDOMNode(this);


findDOMNode no longer behaves the way it used to. This looked like the only change needed to get ads rendering. Whether they are rendering correctly I'll leave to you guys ;) at least to tell me that they're broken.

Engywook2005 · 2023-03-02T15:09:15Z

@stephensauceda I can have a look into the failing unit tests. The first of the two is almost certainly from our having made some tweaks to the out of page ads to accommodate the mobile interstitial, and can most likely be fixed by updating the test.

Will need to get back to you on the second test which is timing out. I wonder if there's something about this under the issues tab under the main repo.

* update deps * update radium to version supporting react 17 * add prepare script * remove prepare script * findDOMNode is deprecated * make peerDependencies more lenient * ref needs to be on different node * Reverting the version as i will do that in another step --------- Co-authored-by: Andrew Sotiriou <[email protected]>

* Enable non-personalized Ads (#1) * Enable non-personalized Ads According to EU regulations, it is mandatory to enable users to opt-out to tracking cookies GPT allows us to respect this law using the `npa` flag on Ads. * Adding npa flag to re-render props * Fix npa variable in componentWillReceiveProps * Increase the bundle-size to accept the new amount of code * Changed the place where NPA is set * Update Bling.js Fixes an issue where `setAttributes` can be called when there is no ad slot * Bump to 2.0.2 * chore: Update package.json Update package.json name and urls after forking * chore: Update API list Update GPT API list to latest * chore: Add prettier config (#3) Add prettier config and fix bracket spacing mismatch * Fixing things * fix: Don't setState on unmounted ad (#5) Don't set state on an unmounted ad. Cribbed from axioscode@9425436. * fix: Fix undefined function if apiNotReady (#6) Fix undefined function if apiNotReady. Cribbed from ticketmaster@5ef19b1. * fix: Replace hasOwnProperty with more reliable check (#7) Replace hasOwnProperty with more reliable check. Cribbed from nfl@8826ee3. * fix: Do not refresh all ads when new component loads (#8) Fixes an issue where all ad slots would reload when syncCorrelator was true and a new slot was added * refactor: Check for updateCorrelator (#9) Check for deprecated updateCorrelator method before calling * build: Update radium (#10) * build: Update radium Update radium to fix "TypeError: Cannot read property 'object' of undefined" when running examples * Use ^ * Fixing things * fix: Stop multiple refresh on mq change (#11) * fix: Stop multiple refresh on mq change Stops multiple calls to instance.refresh() on media query change. This can happen when an instance is associated with multiple instances. * Only debounce this.refresh * Fix comment * Merge it * feat: Add support for custom refresh function (#14) Adds the ability to specify a custom refresh function using `configure`. This function replaces the call to `pubads().refresh`. This is useful for certain header bidding configurations, such as App Nexus that requires to `pbjs.refresh` to be called. * work this time * perf: Use googletag.cmd.push instead of timeout to process queue (#16) Use googletag.cmd.push instead of timeout to check if pubads is ready. Fixes issue with recurring handler. * work this time * Update all instances of googletagservices gpt url to securepubads gpt url (#20) * Update all instances of googletagservices gpt url to securepubads gpt url * Add UNSAFE so we can look to update REACT * update remaining paths; use https to avoid redirect Co-authored-by: Andrew Berry <[email protected]> * update linking instructions; some cleanup (#21) * remove enableServices timeout from GPT mock (#22) * Release 2.1.2 (#24) * Remove deprecated ContentService (#23) * Remove deprecated ContentService usage * Update test * doc updates Co-authored-by: Andrew Sotiriou <[email protected]> * update changelog and version in package.json (#25) * Ton of MI updates (#26) * Ton of MI updates * Minor changes * change release and update changelog (#27) * Gregt/negative viewable threshold (#28) * Can work with negative viewability threshold. * Offset in log remark is now VT for clarity. * Checking for isNaN * removed logging Co-authored-by: Engywook2005 <[email protected]> * Changed version and updated changelog (#29) * Changed version and updated changelog * Fixed typo in package.json Co-authored-by: Engywook2005 <[email protected]> * Andrew s/neg vt issue (#30) * Fixes for neg VT * Update isHidden * Cleanup * Remove log * Update call * Release 2 1 6 (#31) * update packageJSON version * Update changelog * add prepare script * Revert "add prepare script" This reverts commit 49fcf5b. * React v17 support (#32) * update deps * update radium to version supporting react 17 * add prepare script * remove prepare script * findDOMNode is deprecated * make peerDependencies more lenient * ref needs to be on different node * Reverting the version as i will do that in another step --------- Co-authored-by: Andrew Sotiriou <[email protected]> * release branch (#33) * Remove console log (#34) * Update package and remove console log (#35) * Gregt/lazy load (#36) * Add stuff for lazy load * First shot at ignoring isInViewport response. * Will render ads even if the inviewport check fails * Removed commented out line of code * Version bump and changelog update --------- Co-authored-by: Andrew Sotiriou <[email protected]> Co-authored-by: Greg Thorson <[email protected]> --------- Co-authored-by: Carlos A. Gomes <[email protected]> Co-authored-by: Carlos Gomes <[email protected]> Co-authored-by: Michael Blanchard <[email protected]> Co-authored-by: Mike Kruk <[email protected]> Co-authored-by: Andrew Berry <[email protected]> Co-authored-by: Andrew Berry <[email protected]> Co-authored-by: Greg Thorson <[email protected]> Co-authored-by: Engywook2005 <[email protected]> Co-authored-by: Stephen Sauceda <[email protected]> Co-authored-by: Greg Thorson <[email protected]>

update deps

ac34fd7

stephensauceda requested review from andrew-sotiriou and andrewb February 22, 2023 20:06

stephensauceda commented Feb 22, 2023

View reviewed changes

update radium to version supporting react 17

5b258e9

stephensauceda marked this pull request as ready for review February 23, 2023 18:10

stephensauceda added 5 commits February 23, 2023 16:11

add prepare script

faf7870

remove prepare script

cec18b3

findDOMNode is deprecated

939ee3d

make peerDependencies more lenient

2312442

ref needs to be on different node

9d0eec6

stephensauceda requested a review from Engywook2005 March 1, 2023 20:31

stephensauceda commented Mar 1, 2023

View reviewed changes

Reverting the version as i will do that in another step

799f43e

andrew-sotiriou merged commit 80bd4ca into master Mar 3, 2023

andrew-sotiriou deleted the s/react17-support branch January 8, 2024 19:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

React v17 support #32

React v17 support #32

stephensauceda commented Feb 22, 2023 •

edited

Loading

stephensauceda Feb 22, 2023

stephensauceda Feb 22, 2023

stephensauceda Mar 1, 2023

stephensauceda Mar 1, 2023

Engywook2005 commented Mar 2, 2023

React v17 support #32

React v17 support #32

Conversation

stephensauceda commented Feb 22, 2023 • edited Loading

What does this PR do?

How can this change be manually tested?

Asana Card

Questions or concerns about this change?

Deployment Notes

PR Readiness

Code Readiness

Automated Testing

Other Considerations

stephensauceda Feb 22, 2023

Choose a reason for hiding this comment

stephensauceda Feb 22, 2023

Choose a reason for hiding this comment

stephensauceda Mar 1, 2023

Choose a reason for hiding this comment

stephensauceda Mar 1, 2023

Choose a reason for hiding this comment

Engywook2005 commented Mar 2, 2023

stephensauceda commented Feb 22, 2023 •

edited

Loading