Refactor: script injectors #39
Conversation
| window.setTimeout(() => { | ||
| trackPageView(location) | ||
| redBoxLtdTracker && redBoxLtdTracker.push(['page', location.pathname]) |
| </svg> | ||
| ) | ||
|
|
||
| const ScriptInjector: React.FC = () => { |
There was a problem hiding this comment.
this example injects a tracking pixel
| @@ -34,63 +25,55 @@ const Icon: React.FC = () => ( | |||
| </svg> | |||
| ) | |||
|
|
|||
| // ensure that the tracker script will be initialized once in runtime | |||
| let wasInitialized = false | |||
| const ScriptInjector: React.FC = () => { | |||
There was a problem hiding this comment.
this integration injects a script tag and manipulates window as many tracking scripts do
| interface RedBoxLtdTracker extends Array<Array<String>>, Tracker {} | ||
|
|
||
| // For usage non-react context when tracking page views (Docusaurus, Gatsby, ...) | ||
| // @todo we need to save-guard this by checking if integration is actually enabled |
| } | ||
| React.useEffect(() => { | ||
| if (isEnabled && !tracker) { | ||
| locateTracker('rbltd', setTracker) |
There was a problem hiding this comment.
relevant, this ensures react can track to the correct object as soon its available
| @@ -17,6 +17,7 @@ | |||
| "@types/react": "^16.9.11", | |||
| "@types/react-dom": "^16.8.4", | |||
| "parcel": "1.12.3", | |||
| "parcel-plugin-static-files-copy": "^2.6.0", | |||
There was a problem hiding this comment.
relevant, copies our red-box-ltd.js and pixel.png into the example public dir
| Boolean(i.ScriptInjector) | ||
| ) | ||
| .map(({ ScriptInjector, id }) => <ScriptInjector key={id} />) | ||
| }, [config, decisions, isMounted]) |
There was a problem hiding this comment.
relevant, this is the bug fix for multiple wrapper/injector components
| export type Status = 'idle' | 'loading' | 'ready' | 'error' | ||
| export type ScriptElt = HTMLScriptElement | null | ||
|
|
||
| export function useScript( |
There was a problem hiding this comment.
relevant, injects the script tag and removes it when dismounted
| return status | ||
| } | ||
|
|
||
| export function locateTracker( |
There was a problem hiding this comment.
relevant, finds our tracker from the external script in the window object, to ensure user userTracker() hook returns the proper tracker
| @@ -65,8 +65,8 @@ export const IntegrationProfile: React.FC<IntegrationProfileProps> = ({ | |||
| </td> | |||
| </tr> | |||
| <tr> | |||
| <td>WrapperComponent</td> | |||
| <td>{WrapperComponent ? '✅' : '⛔️'}</td> | |||
| <td>Injects <script/> tag</td> | |||
There was a problem hiding this comment.
This makes now more sense :)
| @@ -0,0 +1,33 @@ | |||
| module.exports = { | |||
There was a problem hiding this comment.
relevant, our first eslint rule.
This ensures people don't create integrations using document.createElement('script') to inject script tags. They should use our helper.
axe312ger
force-pushed
the
refactor/script-injectors
branch
from
2f23060 to
0fb0cf0
Compare
There is no need to wrap the whole application with our integration components, thats why we applied #33
This PR refactors the code so it reflects what these integrations actually do:
It also fixes a bug that prevents multiple WrapperComponents to render.