Do not access location during import of tracker module - #6039
Conversation
95aae20 to
31e45ae
Compare
|
I'm not sure if I'm able to get the |
31e45ae to
0dc7090
Compare
|
I updated this PR after the module export changes (#6158) went in. Any chance it can get looked at? |
|
@toofishes Thanks for your contribution! It is a real issue, as the package readme (https://www.npmjs.com/package/@plausible-analytics/tracker) is supportive of SSR, yet it doesn't work with |
|
|
||
| var currentEngagementIgnored | ||
| var currentEngagementURL = location.href | ||
| var currentEngagementURL = '' |
There was a problem hiding this comment.
What happens if this is left undefined?
There was a problem hiding this comment.
That is probably fine as well. I wasn't sure whether to prefer the pattern of currentEngagementProps/currentEngagementMaxScrollDepth (initializing to something of the same type) or currentEngagementIgnored (just leaving undefined). Would you like me to update the PR and change it?
There was a problem hiding this comment.
Roger, I think that was a reasonable approach!
On my end, the main concern is what will happen if this value ends up being sent to the API (for whatever future reason).
I removed the listeningOnEngagement condition in prePageviewTrack (simulating us screwing up in the future) and tested undefined, null and '' values for currentEngagementURL. In all cases, tests started failing (great!).
The u value in the payload was undefined, null or '' respectively. When undefined or null, the api/event endpoint rejects the event with 400. When it was '', the endpoint returns 202, but drops it due to not finding a matching session (x-plausible-dropped: 1).
I think it's better to see the error 400 tracker-side, so I'd prefer it to be uninitialized. If you agree, please update the PR.
I was also a bit surprised to be reporting it! Likely either use of jsdom/happy-dom whether on purpose or accidentally, deferred loading, or conditionally excluding it from SSR? |
For use in a server-side rendered environment, it is important we don't access
browser variables before `init` or `track` are called, that might not exist in
Node/Bun/etc.
There isn't much reason to set this variable to location.href prior to
`postPageviewTrack` being called, as we won't use it until engagements are
registered, in which case, we will have set the URL from the payload.
This is the current error when importing (but not running) the code in Node:
```
Welcome to Node.js v25.4.0.
Type ".help" for more information.
> import('@plausible-analytics/tracker/plausible.js')
Promise {
<pending>,
Symbol(async_id_symbol): 103,
Symbol(trigger_async_id_symbol): 6
}
> Uncaught ReferenceError: location is not defined
```
0dc7090 to
0d9a49e
Compare
|
PR updated to simply leave it undefined by default. |
3 participants
Changes
After making this change, I am able to use server-side generation of a Vue-based website using vite-ssg and the node package, with a static import of the analytics code so it gets inlined and packed into the main JS bundle.
Initialize currentEngagementURL to undefined, not
location.hrefFor use in a server-side rendered environment, it is important we don't access browser variables before
initortrackare called, that might not exist in Node/Bun/etc.There isn't much reason to set this variable to location.href prior to
postPageviewTrackbeing called, as we won't use it until engagements are registered, in which case, we will have set the URL from the payload.This is the current error when importing (but not running) the code in Node:
After this change, the import is successful:
Tests
Changelog
I didn't update the tracker changelog- this seems like something done at release time, when I read the tracker-specific docs.
Documentation
Dark mode