-
Notifications
You must be signed in to change notification settings - Fork 46
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
chore: set locale and timezone using the react-native-localize library #373
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,6 +33,7 @@ | |
"react-native": "^0.69.1", | ||
"react-native-device-info": "^10.3.0", | ||
"react-native-navigation": "^6.0.0", | ||
"react-native-localize": "^3.0.0", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: react-native-localize is added as a devDependency but not as a peerDependency. Since it's replacing core functionality, it should probably be added to peerDependencies with an appropriate version range. |
||
"posthog-react-native-session-replay": "^0.1" | ||
}, | ||
"peerDependencies": { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ import { OptionalExpoFileSystem } from './optional/OptionalExpoFileSystem' | |
import { OptionalExpoLocalization } from './optional/OptionalExpoLocalization' | ||
import { OptionalReactNativeDeviceInfo } from './optional/OptionalReactNativeDeviceInfo' | ||
import { PostHogCustomAppProperties, PostHogCustomStorage } from './types' | ||
import { OptionalReactNativeLocalize } from './optional/OptionalReactNativeLocalize' | ||
|
||
export const getAppProperties = (): PostHogCustomAppProperties => { | ||
let deviceType = 'Mobile' | ||
|
@@ -58,6 +59,27 @@ export const getAppProperties = (): PostHogCustomAppProperties => { | |
if (OptionalExpoLocalization) { | ||
properties.$locale = OptionalExpoLocalization.locale | ||
properties.$timezone = OptionalExpoLocalization.timezone | ||
} else if (OptionalReactNativeLocalize) { | ||
const localesFn = OptionalReactNativeLocalize.getLocales | ||
if (localesFn) { | ||
const locales = localesFn() | ||
|
||
if (locales && locales.length > 0) { | ||
const languageTag = locales[0].languageTag | ||
if (languageTag) { | ||
properties.$locale = languageTag | ||
} | ||
} | ||
} | ||
Comment on lines
+63
to
+73
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Consider adding error handling for localesFn() call which could throw an exception |
||
|
||
const timezoneFn = OptionalReactNativeLocalize.getTimeZone | ||
if (timezoneFn) { | ||
const timezone = timezoneFn() | ||
|
||
if (timezone) { | ||
properties.$timezone = timezone | ||
} | ||
} | ||
Comment on lines
+75
to
+82
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Consider adding error handling for timezoneFn() call which could throw an exception |
||
} | ||
|
||
return properties | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
interface ReactNativeLocalize { | ||
getLocales: () => { | ||
languageCode: string | ||
scriptCode?: string | ||
countryCode: string | ||
languageTag: string | ||
isRTL: boolean | ||
}[] | ||
|
||
getTimeZone(): string | ||
} | ||
|
||
import type ReactNativeLocalize from 'react-native-localize' | ||
|
||
export let OptionalReactNativeLocalize: typeof ReactNativeLocalize | undefined = undefined | ||
|
||
// web support requires webpack | ||
// https://github.com/zoontek/react-native-localize#web-support | ||
try { | ||
OptionalReactNativeLocalize = require('react-native-localize') | ||
} catch (e) {} | ||
Comment on lines
+20
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Empty catch block could silently fail. Consider logging the error when in debug mode. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should have stayed let - its gets reasigned under
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the linter did it automatically btw (eslint, prettier), but as I can see it, the usage props are resigned, but not the
usage
itself.Happy to change it back but am I missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the 2 openai
usage
gets reasigned, but not the anthropic one.