feat: Convert event_tags_validator module to TS #528
Conversation
| @@ -30,6 +30,8 @@ const MODULE_NAME = 'USER_PROFILE_SERVICE_VALIDATOR'; | |||
| * @return {boolean} true if the instance is valid | |||
| * @throws If the instance is not valid | |||
| */ | |||
|
|
|||
| //TODO: Use defined UserProfileService interface instead of 'Record<string, unknown>' | |||
There was a problem hiding this comment.
UserProfileService interface is declared in '@optimizely/optimizely-sdk' molude in index.d.ts, so I tried importing it with
{ UserProfileService } from '@optimizely/optimizely-sdk' and also import { UserProfileService } from '../../index', which did not work. Any suggestions on how to import it would be much appreciated! @mjc1283 @mikeng13
There was a problem hiding this comment.
Hmm yeah I think we'll have to do something a little more drastic. Like the UserProfileService interface needs to be defined in one of the .ts files, maybe it can be its own file under plugins and then the top level index.d.ts would import that interface and re-export. @mjc1283 wdyt?
There was a problem hiding this comment.
For me, import { UserProfileService } from '@optimizely/optimizely-sdk'; works, @yavorona we can dig to this to figure out what's wrong. Maybe it's related to IDE configuration? I was able to get it to compile with npm run build.
But in any case, I don't think we want to use that UserProfileService type here. Again I think unknown is what we need, treating this as an unvalidated value (the user could have passed in anything).
@mikeng13
Like the UserProfileService interface needs to be defined in one of the .ts files, maybe it can be its own file under plugins and then the top level index.d.ts would import that interface and re-export.
I think this will work. We could only export types at the top level that consumers would need, so not everything.
There was a problem hiding this comment.
For me,
import { UserProfileService } from '@optimizely/optimizely-sdk';works, @yavorona we can dig to this to figure out what's wrong. Maybe it's related to IDE configuration? I was able to get it to compile withnpm run build.
Strange, even after compiling, it still Cannot find module '@optimizely/optimizely-sdk'. 🤔 I will do more research on that.
There was a problem hiding this comment.
But in any case, I don't think we want to use that
UserProfileServicetype here. Again I thinkunknownis what we need, treating this as an unvalidated value (the user could have passed in anything).
Changing it to unknown produces TSError: ⨯ Unable to compile TypeScript: for both if (typeof userProfileServiceInstance.lookup !== 'function') {...} and else if (typeof userProfileServiceInstance.save !== 'function') {...} . any in this case works though, so it might make sense to use it here. @mjc1283 what do you think?
optimizely-codereview-bot
added
🤖 4 hrs
🤖 8 hrs
🤖 12 hrs
and removed
🤖 4 hrs
🤖 8 hrs
labels
| * @throws If event tags are not valid | ||
| */ | ||
| export var validate = function(eventTags) { | ||
| export function validate(eventTags: EventTags): boolean { |
There was a problem hiding this comment.
I think the right type for eventTags is unknown. This is a value passed in by the user of unknown type.
| @@ -30,6 +30,8 @@ const MODULE_NAME = 'USER_PROFILE_SERVICE_VALIDATOR'; | |||
| * @return {boolean} true if the instance is valid | |||
| * @throws If the instance is not valid | |||
| */ | |||
|
|
|||
| //TODO: Use defined UserProfileService interface instead of 'Record<string, unknown>' | |||
There was a problem hiding this comment.
For me, import { UserProfileService } from '@optimizely/optimizely-sdk'; works, @yavorona we can dig to this to figure out what's wrong. Maybe it's related to IDE configuration? I was able to get it to compile with npm run build.
But in any case, I don't think we want to use that UserProfileService type here. Again I think unknown is what we need, treating this as an unvalidated value (the user could have passed in anything).
@mikeng13
Like the UserProfileService interface needs to be defined in one of the .ts files, maybe it can be its own file under plugins and then the top level index.d.ts would import that interface and re-export.
I think this will work. We could only export types at the top level that consumers would need, so not everything.
yavorona
force-pushed
the
pnguen/convert-event-tags-validator-to-ts
branch
from
e864140 to
f94a323
Compare
Summary: Convert event_tags_validator module from JS to TS Revise user_profile_service_validator module Test plan: Existing unit tests
Summary
event_tags_validatormodule from JS to TSuser_profile_service_validatormoduleTest plan