Fix query string bug #888
Fix query string bug #888
Conversation
🦋 Changeset detectedLatest commit: d456302 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@danieljackins page.search should always be a string https://segment.com/docs/connections/spec/page/#properties? |
Right, but this bug report was from a customer who was passing in an object to it in analytics-classic (and older versions of analytics-next) successfully. |
| @@ -131,7 +131,8 @@ export function normalize( | |||
| const ctx = json.context | |||
|
|
|||
| // This guard against missing ctx.page should not be neccessary, since context.page is always defined | |||
| const query: string = ctx.page?.search || '' | |||
There was a problem hiding this comment.
Not sure where we set search for ourselves, but if it's internal maybe we should try putting our stuff in a particular nested object internal? or use _search ?
| @@ -131,7 +131,8 @@ export function normalize( | |||
| const ctx = json.context | |||
|
|
|||
| // This guard against missing ctx.page should not be neccessary, since context.page is always defined | |||
| const query: string = ctx.page?.search || '' | |||
| const query: string = | |||
| typeof ctx.page?.search === 'string' ? ctx.page?.search : '' | |||
There was a problem hiding this comment.
This behavior needs a test, correct? 'Page calls should be able to accept search as an object.'
Since we started looking at the ctx object for the query string, if a user passed in an object for the 'search' key an error would be thrown from trying to parse a string. This was causing these events to be dropped.
Only override context.page with event properties on page calls
[] I've included a changeset (psst. run
yarn changeset. Read about changesets here).