Merge pull request #455 from Giphy/bottle-data-fix-2

bottle data tags are urls
Giphy · Nov 1, 2024 · 2a05915 · 2a05915
2 parents 91f751d + 43c2969
commit 2a05915
Show file tree

Hide file tree

Showing 10 changed files with 779 additions and 214 deletions.
diff --git a/.changeset/cold-bananas-serve.md b/.changeset/cold-bananas-serve.md
@@ -0,0 +1,5 @@
+---
+'@giphy/react-components': patch
+---
+
+change bottle tag approach
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -11,12 +11,12 @@ jobs:
             TURBO_TOKEN: ${{ secrets.TURBO_TOKEN }}
             TURBO_TEAM: giphy-web
         steps:
-            - uses: actions/checkout@v3
-            - uses: actions/setup-node@v3
+            - uses: actions/checkout@v4
+            - uses: actions/setup-node@v4
               with:
-                  node-version: 16.x
+                  node-version: 18.x
                   cache: 'yarn'
-            - run: yarn install --immutable 
+            - run: yarn install --immutable
             - name: Build
               run: yarn run build
             - name: Lint and Test

diff --git a/.gitignore b/.gitignore
@@ -24,3 +24,4 @@ umd/
 !.yarn/versions
 .hintrc
 .vscode/
+.zed/
diff --git a/packages/react-components/cypress/utils/video-test-utils.ts b/packages/react-components/cypress/utils/video-test-utils.ts
@@ -93,10 +93,10 @@ export function checkVideoEvents(ctx: VideoTestUtilsContext) {
         .then(async (elems) => {
             const el = elems.get(0)
             await el.play()
-            return new Promise((resolve) => {
+            return new Promise<void>((resolve) => {
                 function listener() {
                     el.removeEventListener('ended', listener)
-                    resolve(null)
+                    resolve()
                 }
                 el.addEventListener('ended', listener)
             })

diff --git a/packages/react-components/package.json b/packages/react-components/package.json
@@ -16,8 +16,8 @@
         "build-storybook": "storybook build"
     },
     "devDependencies": {
-        "@babel/core": "^7.18.6",
-        "@cypress/react18": "^2.0.0",
+        "@babel/core": "^7.26.0",
+        "@cypress/react18": "^2.0.1",
         "@percy/cli": "^1.16.0",
         "@percy/cypress": "^3.1.2",
         "@percy/storybook": "4.3.6",
@@ -38,17 +38,17 @@
         "@types/react": "^18.2.79",
         "@types/react-dom": "^18.2.25",
         "@types/storybook__react": "^5.2.1",
-        "@types/throttle-debounce": "^2.1.0",
+        "@types/throttle-debounce": "^5.0.2",
         "awesome-typescript-loader": "^5.2.1",
-        "babel-loader": "^8.2.5",
-        "cypress": "^12.17.2",
+        "babel-loader": "^9.2.1",
+        "cypress": "^13.15.1",
         "fetch-mock": "^9.11.0",
-        "react": "^18.2.0",
-        "react-dom": "^18.2.0",
+        "react": "^18.3.1",
+        "react-dom": "^18.3.1",
         "storybook": "^7.1.0",
         "styled-components": "^6.0.7",
         "ts-loader": "^9.3.1",
-        "typescript": "^5.0.4",
+        "typescript": "^5.6.3",
         "webpack": "^5.74.0"
     },
     "name": "@giphy/react-components",

diff --git a/packages/react-components/src/components/bottle-data.tsx b/packages/react-components/src/components/bottle-data.tsx
@@ -1,5 +1,5 @@
 import { noUUIDRandom } from '@giphy/js-util'
-import React, { useRef, useLayoutEffect } from 'react'
+import React from 'react'
 
 function processTag(tag: string) {
     tag = tag.replace(`%%CACHEBUSTER%%`, noUUIDRandom())
@@ -11,26 +11,8 @@ function processTag(tag: string) {
     }
     return tag
 }
-/**
- * Execute a script tag in a React component.
- * https://macarthur.me/posts/script-tags-in-react/
- */
-function BottleData({ markup }: { markup: string }) {
-    const elRef = useRef<HTMLDivElement>(null)
-
-    useLayoutEffect(() => {
-        if (!elRef.current) return
-        const range = document.createRange()
-        range.selectNode(elRef.current)
-        const documentFragment = range.createContextualFragment(processTag(markup))
-
-        // Inject the markup, triggering a re-run!
-        elRef.current.innerHTML = ''
-        elRef.current.append(documentFragment)
-        // eslint-disable-next-line react-hooks/exhaustive-deps
-    }, [])
-
-    return <div ref={elRef} dangerouslySetInnerHTML={{ __html: markup }}></div>
+function BottleData({ src }: { src: string }) {
+    return <img src={processTag(src)} />
 }
 
 export default BottleData
diff --git a/packages/react-components/src/components/gif.tsx b/packages/react-components/src/components/gif.tsx
@@ -239,6 +239,14 @@ const Gif = ({
         percentHeight = `${ratio}%`
     }
     const bestRendition = getBestRendition(gif.images, width, height)
+    if (!bestRendition) {
+        if (gif.images) {
+            console.error(`no rendition for ${gif.id}, rendition names: ${Object.keys(gif.images).join(', ')}`)
+        } else {
+            console.error(`no rendition for ${gif.id} - no images`)
+        }
+        return null
+    }
     const rendition = gif.images[bestRendition.renditionName] as ImageAllTypes
     const background =
         backgroundColor || // <- specified background prop
@@ -287,7 +295,7 @@ const Gif = ({
                     alt={getAltText(gif)}
                     onLoad={shouldShowMedia ? onImageLoad : () => {}}
                 />
-                {isAd && bottleData?.tags?.map((tag: string, index: number) => <BottleData markup={tag} key={index} />)}
+                {isAd && bottleData?.tags?.map((tag: string, index: number) => <BottleData src={tag} key={index} />)}
             </picture>
             {Overlay && (
                 // only render the overlay on the client since it depends on shouldShowMedia

diff --git a/packages/react-components/stories/gif.stories.tsx b/packages/react-components/stories/gif.stories.tsx
@@ -108,20 +108,16 @@ export const GifWithAd: Story = {
         bottle_data: {
             tid: '8691c382d3eed000da83ecc2ceef747b91190ab0e5bc0bc95ff5c80eeda242fa',
             tags: [
-                `<noscript class="MOAT-giphydisplay879451385633?moatClientLevel1=ff6046d1-7125-462a-809c-efa338464775&amp;moatClientLevel2=e66d47a2-1b13-4b74-a34d-23fbdc10ddb5&amp;moatClientLevel3=82&amp;moatClientLevel4=3o7WIw8TV4UJROSElW&amp;moatClientSlicer1=e26c89fe-fb01-4442-9e8f-37940600265c&amp;moatClientSlicer2=giphytrending"></noscript>
-                <script type="text/javascript">
-              console.log('cache:%%CACHEBUSTER%%')
-              console.log('timestamp:%%TIMESTAMP%%')
-              console.log('window size:%%APP_WINDOW_SIZE%%')
-              console.log('lang:%%DEVICE_LANGUAGE%%')
-            </script>`,
+                `https://www.giphy.com/test.js?cache=%%CACHEBUSTER%%&ts=%%TIMESTAMP%%&window=%%APP_WINDOW_SIZE%%&lang=%%DEVICE_LANGUAGE%%`,
+                `https://www.giphy.com/test2.js?cache=%%CACHEBUSTER%%&ts=%%TIMESTAMP%%&window=%%APP_WINDOW_SIZE%%&lang=%%DEVICE_LANGUAGE%%`,
             ],
         },
     },
 }
 
 export const GifThatStretches: Story = {
     args: {
+        id: 'sTczweWUTxLqg',
         percentWidth: '50%',
     },
 }

diff --git a/packages/svelte-components/package.json b/packages/svelte-components/package.json
@@ -7,8 +7,8 @@
         "preview": "vite preview",
         "package": "svelte-kit sync && svelte-package && publint",
         "prepublishOnly": "npm run package",
-        "lint": "run -T eslint . --ext .ts,.tsx,.svelte",
-        "test": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json",
+        "lint": "yarn build && run -T eslint . --ext .ts,.tsx,.svelte",
+        "test": "yarn build && svelte-kit sync && svelte-check --tsconfig ./tsconfig.json",
         "check:watch": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json --watch"
     },
     "description": "GIPHY components for Svelte",
-Original file line number
+Diff line change
@@ Expand Up / @@ -24,3 +24,4 @@ umd/ @@
     !.yarn/versions
     .hintrc
     .vscode/
+    .zed/