-
Notifications
You must be signed in to change notification settings - Fork 34
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
Scripts? #49
Comments
react-head will load it twice too. It's better to use another way of loading for scripts IMO. @tizmagik What do you think? |
It is very nice to be able to specify head content without having to
special case it in the SSR renderer - then the renderer can be a
package and your project has less code.
|
Interesting! I think maybe we could figure out a way to support this, either by no-op’ing on the server or no-op’ing on the client perhaps? @wmertens is your use case for inline scripts or linked scripts, or both? If you have a specific use case or some sample code snippets or something you could share that would be helpful to understand the specific use case better. Will play around with some ideas and post back when I get a chance. |
The use case is both inline and linked scripts. The most important thing is that the scripts should only execute once when already rendered via SSR. I suppose checking the contents would be sufficient to ward against this? Here's how I load ga: /* global __PRODUCTION__ __CLIENT__ __CONFIG__ */
import React, {Fragment} from 'react'
import Helmet from 'react-helmet'
import {LocationListener} from 'plugins/react-router-deluxe'
const {gaId, gAdwordsId} = __CONFIG__
export const gtag = (...args) => global.gtag && global.gtag(...args)
const handlePageView = location =>
gtag('config', gaId, {
// eslint-disable-next-line camelcase
page_path: location.pathname + location.search,
})
// Don't render the scripts on the client, they
// should be loaded on initial html only
// Also https://github.com/nfl/react-helmet/issues/357
const GoogleAnalytics = () =>
__CLIENT__ ? (
<LocationListener onChange={handlePageView} />
) : __PRODUCTION__ && gaId ? (
<Fragment>
<Helmet>
{/* Putting in head because it also offers page speed tracking */}
<script
async
src={`https://www.googletagmanager.com/gtag/js?id=${gaId}`}
/>
<script>
{// Must be string to prevent React from messing with the code
`window.dataLayer=window.dataLayer||[];function gtag(){dataLayer.push(arguments);}gtag('js',new Date());gtag('config','${gaId}');${
gAdwordsId ? `gtag('config','${gAdwordsId}');` : ''
}`}
</script>
</Helmet>
</Fragment>
) : (
false
)
export default GoogleAnalytics and then I can just put I think this might also be useful on the client for when you need a third-party library: put a |
@tizmagik Would you still be game to take a PR on this? I might be interested. |
@RichiCoder1 — yea for sure, if you’re up for it! Thanks in advance 🙏 |
I'm also looking for this functionality and willing to contribute, but I'm not sure the solution to the double load. For external scripts, caching should make it irrelevant, right? Inlined scripts are problematic though. We can't not render them on the client or they won't be set when people navigate in the client. Maybe the server could pass some data to the window indicating an inline script is already loaded? |
Inline scripts could be wrapped with something that makes sure they only run once, but that seems hard to do. |
Oh and for external scripts, caching will indeed prevent downloads, but it will still run the script every time. Most scripts already protect against that, so no big issue, and I can't think of workarounds that always work and don't impact preloading etc. |
Another use case for Is there a need to special case these, or can a |
For |
I noticed that there is no support for script tags. This is useful for e.g. conditionally including trackers in SSR.
React-helmet can do it, but unfortunately it loads the script twice on SSR if you also render it on the client.
The text was updated successfully, but these errors were encountered: