chore: add surveys device types check #1698

marandaneto · 2025-01-28T21:45:40Z

Changes

Draft for PostHog/posthog#24495 (SDK side)
docs PostHog/posthog.com#10529

...

Checklist

Tests for new code (see advice on the tests we use)
Accounted for the impact of any changes across different browsers
Accounted for backwards compatibility of any changes (no breaking changes in posthog-js!)

vercel · 2025-01-28T21:45:44Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
posthog-js	✅ Ready (Inspect)	Visit Preview	Jan 29, 2025 11:58pm

github-actions · 2025-01-28T21:49:56Z

Size Change: +5.86 kB (+0.18%)

Total Size: 3.29 MB

Filename	Size	Change
`dist/all-external-dependencies.js`	215 kB	+441 B (+0.21%)
`dist/array.full.es5.js`	266 kB	+430 B (+0.16%)
`dist/array.full.js`	369 kB	+340 B (+0.09%)
`dist/array.full.no-external.js`	368 kB	+340 B (+0.09%)
`dist/array.js`	182 kB	+512 B (+0.28%)
`dist/array.no-external.js`	180 kB	+352 B (+0.2%)
`dist/dead-clicks-autocapture.js`	14.5 kB	+4 B (+0.03%)
`dist/main.js`	183 kB	+512 B (+0.28%)
`dist/module.full.js`	369 kB	+340 B (+0.09%)
`dist/module.full.no-external.js`	368 kB	+340 B (+0.09%)
`dist/module.js`	182 kB	+565 B (+0.31%)
`dist/module.no-external.js`	180 kB	+352 B (+0.2%)
`dist/surveys-preview.js`	69.4 kB	+668 B (+0.97%)
`dist/surveys.js`	72.4 kB	+668 B (+0.93%)

ℹ️ View Unchanged

Filename	Size
`dist/customizations.full.js`	13.8 kB
`dist/exception-autocapture.js`	9.48 kB
`dist/external-scripts-loader.js`	2.64 kB
`dist/recorder-v2.js`	115 kB
`dist/recorder.js`	115 kB
`dist/tracing-headers.js`	1.76 kB
`dist/web-vitals.js`	10.4 kB

_{compressed-size-action}

marandaneto · 2025-01-28T21:55:51Z

src/posthog-surveys-types.ts

@@ -123,7 +123,7 @@ export interface SurveyResponse {

 export type SurveyCallback = (surveys: Survey[]) => void

-export type SurveyUrlMatchType = 'regex' | 'not_regex' | 'exact' | 'is_not' | 'icontains' | 'not_icontains'
+export type SurveyMatchType = 'regex' | 'not_regex' | 'exact' | 'is_not' | 'icontains' | 'not_icontains'


i think we could rename this to something generic and kill WebExperimentUrlMatchType as well

but then this needs to be moved out of the surveys bundle

I think this makes sense. However, since it's generic, I'd say we name it UrlMatchType and move it to src/types.ts

No Url in its name, since its not used only for Urls

oooh, we were working towards something similar in replay 👀 🤔

URL matching and the like

src/posthog-surveys.ts

pauldambra · 2025-01-29T18:53:38Z

src/utils/request-utils.ts

@@ -22,9 +22,9 @@ export const convertToURL = (url: string): HTMLAnchorElement | null => {
    return location
 }

-export const isUrlMatchingRegex = function (url: string, pattern: string): boolean {
+export const isMatchingRegex = function (value: string, pattern: string): boolean {


nit: feels more like string utils than request utils

especially now it's more obviously got nothing to do with URLs

makes sense to move it now

pauldambra · 2025-01-29T18:56:14Z

added a very low context review pass since I was here anyway after being tagged :)

greptile-apps

PR Summary

This PR adds device type targeting capabilities to PostHog surveys, allowing surveys to be conditionally shown based on the user's device type. Here are the key changes:

Added device type targeting in posthog-surveys-types.ts with new fields deviceTypes and deviceTypesMatchType to the Survey interface
Refactored string matching utilities by moving isMatchingRegex from request-utils.ts to string-utils.ts for better code organization
Modified surveyValidationMap in posthog-surveys.ts to handle both URL and device type matching with consistent validation logic
Added doesSurveyDeviceTypesMatch function to check device type conditions against user agent
Updated tests to verify device type targeting works correctly for mobile and web devices

The changes maintain backward compatibility while adding new functionality to target surveys based on device types like Mobile, Web, Android and iOS.

_{💡 (2/5) Greptile learns from your feedback when you react with 👍/👎!}

_{8 file(s) reviewed, 8 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

src/posthog-surveys-types.ts

greptile-apps · 2025-01-29T23:30:41Z

src/extensions/surveys/action-matcher.ts

@@ -121,7 +121,7 @@ export class ActionMatcher {
    private static matchString(url: string, pattern: string, matching: ActionStepStringMatching): boolean {
        switch (matching) {
            case 'regex':
-                return !!window && isUrlMatchingRegex(url, pattern)
+                return !!window && isMatchingRegex(url, pattern)


style: window check is redundant since isMatchingRegex already handles null/undefined cases

greptile-apps · 2025-01-29T23:30:42Z

src/extensions/surveys/action-matcher.ts

                const adjustedRegExpStringPattern = ActionMatcher.escapeStringRegexp(pattern)
                    .replace(/_/g, '.')
                    .replace(/%/g, '.*')
-                return isUrlMatchingRegex(url, adjustedRegExpStringPattern)
+                return isMatchingRegex(url, adjustedRegExpStringPattern)


logic: no boundary checks on the regex pattern after SQL LIKE conversion - could lead to partial matches where full matches were intended

Suggested change

const adjustedRegExpStringPattern = ActionMatcher.escapeStringRegexp(pattern)

.replace(/_/g, '.')

.replace(/%/g, '.*')

return isUrlMatchingRegex(url, adjustedRegExpStringPattern)

return isMatchingRegex(url, adjustedRegExpStringPattern)

const adjustedRegExpStringPattern = '^' + ActionMatcher.escapeStringRegexp(pattern)

.replace(/_/g, '.')

.replace(/%/g, '.*') + '$'

return isMatchingRegex(url, adjustedRegExpStringPattern)

src/posthog-surveys.ts

greptile-apps · 2025-01-29T23:31:02Z

src/utils/string-utils.ts

@@ -1,3 +1,5 @@
+import { isValidRegex } from '.'


style: Import from '.' is ambiguous and could break if file structure changes. Consider using explicit path to the file containing isValidRegex

greptile-apps · 2025-01-29T23:31:02Z

src/utils/string-utils.ts

+    if (!isValidRegex(pattern)) return false
+    return new RegExp(pattern).test(value)


style: Creating new RegExp on every call is inefficient for repeated patterns. Consider caching compiled regex objects

src/utils/string-utils.ts

lucasheriques

looks good!

we already have tests in place for URL matching right?

chore: add device types check

404f8f4

fix

81e230a

vercel bot deployed to Preview January 28, 2025 21:52 View deployment

ref

5d7a0f0

marandaneto commented Jan 28, 2025

View reviewed changes

src/posthog-surveys.ts Outdated Show resolved Hide resolved

vercel bot deployed to Preview January 28, 2025 21:57 View deployment

fix

68545fc

vercel bot deployed to Preview January 29, 2025 00:26 View deployment

tests

5fbb274

vercel bot deployed to Preview January 29, 2025 00:49 View deployment

pauldambra reviewed Jan 29, 2025

View reviewed changes

fix

12ae0d7

vercel bot deployed to Preview January 29, 2025 19:26 View deployment

marandaneto mentioned this pull request Jan 29, 2025

chore: document device type targeting surveys PostHog/posthog.com#10529

Merged

11 tasks

marandaneto marked this pull request as ready for review January 29, 2025 23:29

marandaneto requested a review from a team as a code owner January 29, 2025 23:29

marandaneto requested a review from lucasheriques January 29, 2025 23:29

greptile-apps bot reviewed Jan 29, 2025

View reviewed changes

fix

bf523c1

vercel bot deployed to Preview January 29, 2025 23:58 View deployment

lucasheriques mentioned this pull request Jan 30, 2025

chore: add device type filtering on surveys PostHog/posthog#28109

Merged

lucasheriques approved these changes Jan 30, 2025

View reviewed changes

marandaneto added the bump minor Bump minor version when this PR gets merged label Jan 30, 2025

marandaneto merged commit 9bf2e35 into main Jan 30, 2025
31 checks passed

marandaneto deleted the chore/device-types-local branch January 30, 2025 19:55

marandaneto changed the title ~~chore: add device types check~~ chore: add surveys device types check Jan 30, 2025

marandaneto mentioned this pull request Feb 19, 2025

fix: Remove circular dependency #1750

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: add surveys device types check #1698

chore: add surveys device types check #1698

marandaneto commented Jan 28, 2025 •

edited

Loading

vercel bot commented Jan 28, 2025 •

edited

Loading

github-actions bot commented Jan 28, 2025 •

edited

Loading

marandaneto Jan 28, 2025

marandaneto Jan 28, 2025

lucasheriques Jan 29, 2025

marandaneto Jan 29, 2025

pauldambra Jan 29, 2025

pauldambra Jan 29, 2025

pauldambra Jan 29, 2025

marandaneto Jan 29, 2025

pauldambra commented Jan 29, 2025

greptile-apps bot left a comment

greptile-apps bot Jan 29, 2025

greptile-apps bot Jan 29, 2025

greptile-apps bot Jan 29, 2025

greptile-apps bot Jan 29, 2025

lucasheriques left a comment

		if (!isValidRegex(pattern)) return false
		return new RegExp(pattern).test(value)

chore: add surveys device types check #1698

chore: add surveys device types check #1698

Conversation

marandaneto commented Jan 28, 2025 • edited Loading

Changes

Checklist

vercel bot commented Jan 28, 2025 • edited Loading

github-actions bot commented Jan 28, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pauldambra commented Jan 29, 2025

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot Jan 29, 2025

Choose a reason for hiding this comment

greptile-apps bot Jan 29, 2025

Choose a reason for hiding this comment

greptile-apps bot Jan 29, 2025

Choose a reason for hiding this comment

greptile-apps bot Jan 29, 2025

Choose a reason for hiding this comment

lucasheriques left a comment

Choose a reason for hiding this comment

marandaneto commented Jan 28, 2025 •

edited

Loading

vercel bot commented Jan 28, 2025 •

edited

Loading

github-actions bot commented Jan 28, 2025 •

edited

Loading