feat: [Nt 2734] merge both use personalization and use analytics into use optimization#178
Conversation
- Update hooks and providers to use the new context shape, add useAnalytics and usePersonalization helpers, and introduce OptimizationSdk type for improved type safety.
Lotfi Anwar L Arif (Lotfi-Arif)
changed the title
Lotfi Anwar L Arif (Lotfi-Arif)
requested review from
Alex Freas (akfreas) and
Charles Hudson (phobetron)
as code owners
There was a problem hiding this comment.
These types all seem redundant. I'm thinking we should figure out how to eliminate the need for this file altogether. And if we need a type for something that is defined concretely, we can simply say typeof Something, unless that something isn't intended to be inlined in the generated bundle at all, but I think that's not likely for these types here.
| destroy: () => { | ||
| sdk.destroy() | ||
| }, |
There was a problem hiding this comment.
I think this should require a direct reference to sdk since it probably shouldn't normally be called by integrators.
| reset: () => { | ||
| sdk.reset() | ||
| }, |
There was a problem hiding this comment.
I think this should require a direct reference to sdk since it probably shouldn't normally be called by integrators.
| resolveEntryData(sdk, entry, selectedPersonalizations).entry, | ||
| resolveEntryData: (entry, selectedPersonalizations) => | ||
| resolveEntryData(sdk, entry, selectedPersonalizations), | ||
| states: sdk.states, |
There was a problem hiding this comment.
We've already got the state hooks, so we shouldn't include this here. If someone needs to access it differently for some reason, they can go through sdk.
| trackClick: async (payload) => { | ||
| await sdk.trackClick(payload) | ||
| }, | ||
| trackView: async (payload) => await sdk.trackView(payload), |
There was a problem hiding this comment.
I'm a bit torn on these... we have automatic and semi-automatic tracking handled in the tracking API, and it shouldn't be very common for integrators to call these directly. Let's try removing these and requiring integrators to drop to sdk if they need to call these manually for some reason, and see how it turns out in the beta. I'd hate for customers to start using odd patterns.
| resolveEntryData: (entry, selectedPersonalizations) => | ||
| resolveEntryData(sdk, entry, selectedPersonalizations), | ||
| states: sdk.states, | ||
| tracking: sdk.tracking, |
There was a problem hiding this comment.
I see now that this name might be overly-broad and confusing, especially with the track method right there on the next line. Let's rename it interactionTracking here, and I'll add this to my things-to-rename list in the Web SDK.
There was a problem hiding this comment.
The test files should be renamed to match the files they're testing. It's also a bit odd that we have two test files for the same thing... is there a way to merge them, and remove any duplicate or unnecessary tests so the test file isn't too crazy big?
| export function createTestEntry(id: string): Entry { | ||
| return { | ||
| fields: { title: id }, | ||
| metadata: { tags: [] }, | ||
| sys: { | ||
| contentType: { sys: { id: 'test-content-type', linkType: 'ContentType', type: 'Link' } }, | ||
| createdAt: '2024-01-01T00:00:00.000Z', | ||
| environment: { sys: { id: 'main', linkType: 'Environment', type: 'Link' } }, | ||
| id, | ||
| publishedVersion: 1, | ||
| revision: 1, | ||
| space: { sys: { id: 'space-id', linkType: 'Space', type: 'Link' } }, | ||
| type: 'Entry', | ||
| updatedAt: '2024-01-01T00:00:00.000Z', | ||
| }, | ||
| } | ||
| } |
There was a problem hiding this comment.
We should be able to get fixtures like this out of mocks. But we have fixtures sprinkled everywhere, so if there isn't already a good example of how to do get test fixtures out of mocks then we can leave this for now and that can be a beta or post-beta maintenance topic.
There was a problem hiding this comment.
We should probably rename the directory this stuff is in... maybe optimized-entry since it's all related to that now.
|
Make sure to fix those linter issues |
2 participants
Aligned the React web SDK API with the intended SDK surface by making useOptimization() the main high-level hook for tracking and entry resolution, while adding useOptimizationContext() for readiness/error state and useOptimizedEntry() as the public personalization hook. Also updated OptimizationProvider to support injected SDK instances, refactored OptimizedEntry onto the new hook, refreshed docs/tests, and verified format, lint, typecheck, and unit tests all pass.