Skip to content

Conversation

@axe312ger
Copy link
Collaborator

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:

  • Inject Scripts

It also fixes a bug that prevents multiple WrapperComponents to render.

// @todo find proper solution to ensure page view is tracked after tracker api is initialized
window.setTimeout(() => {
trackPageView(location)
redBoxLtdTracker && redBoxLtdTracker.push(['page', location.pathname])
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

relevant

</svg>
)

const ScriptInjector: React.FC = () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this example injects a tracking pixel


// ensure that the tracker script will be initialized once in runtime
let wasInitialized = false
const ScriptInjector: React.FC = () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@todo 🤔

}
React.useEffect(() => {
if (isEnabled && !tracker) {
locateTracker('rbltd', setTracker)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

relevant, this ensures react can track to the correct object as soon its available

"@types/react": "^16.9.11",
"@types/react-dom": "^16.8.4",
"parcel": "1.12.3",
"parcel-plugin-static-files-copy": "^2.6.0",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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])
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

relevant, injects the script tag and removes it when dismounted

return status
}

export function locateTracker(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

relevant, finds our tracker from the external script in the window object, to ensure user userTracker() hook returns the proper tracker

<tr>
<td>WrapperComponent</td>
<td>{WrapperComponent ? '✅' : '⛔️'}</td>
<td>Injects &lt;script/&gt; tag</td>
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes now more sense :)

@@ -0,0 +1,33 @@
module.exports = {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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.

2 participants