refactor: Replaced explicit typings with auto generated ones #745

ozayr-zaviar · 2022-02-25T15:06:06Z

Summary

This SDK was originally javascript only. We had to add explicit typings in lib/index.d.ts to support typescript users. Now that it has been converted to typescript, we can safely remove lib/index.d.ts and autogenerate typings. This PR does exactly that. We also encountered some discrepancies with existing typings so we fixed those too.

Test plan

Manually tested thoroughly with typescript projects
All existing tests pass.

coveralls · 2022-02-25T15:14:17Z

Coverage increased (+0.001%) to 97.185% when pulling 10fcd43 on uzair/lite-typings into 5d526bd on master.

zashraf1985

This looks good so far. Requested a few more changes

zashraf1985 · 2022-03-22T16:45:53Z

packages/optimizely-sdk/lib/export_types.ts

+  EventDispatcher,
+  DatafileOptions,
+  SDKOptions,
+  OptimizelyOptions,


SDKOptions and OptimizelyOptions are not needed to be exposed. Config is enough for external usage

zashraf1985 · 2022-03-22T16:49:37Z

packages/optimizely-sdk/lib/shared_types.ts

+  eventTags: EventTags;
+  logEvent: Event;
+}
+
 /**
 * Entry level Config Entities
 */
 export interface SDKOptions {


Can you compare it with Config and remove it if not needed. this looks redundant and might not be needed anymore.

Config and SDKOptions shares most of the same variables except for:

Config:

Extra jsonSchemaValidator variable in interface.

Instead of using LogLevel | string, uses union type on all LOG_LEVEL enums.

SDKOptions contains:

Extra eventMaxQueueSize variable in interface.

Extra isValidInstance variable in interface.

We should consider having a base interface both Config and SDKOptions extend.

thanks for pointing these out. WE are consolidating these two types in to one because Config was exposed from the explicit typings while SDKOptions was being used in the actual code. We need to use one of those now because we are removing the explicit types. We will keep Config and remove SDKOptions because thats what was being exposed to typescript users. I have requested @ozayr-zaviar to add the missing variables mentioned in your comments.

- fixed some lint warnings - Added changelog

zashraf1985

LGMT! Great work @ozayr-zaviar

zashraf1985 · 2022-03-24T21:17:46Z

@opti-jnguyen It will be great if we can get your review on this whenever you find some time.

This reverts commit 90e4149.

opti-jnguyen · 2022-03-25T00:07:13Z

packages/optimizely-sdk/lib/plugins/datafile_manager/http_polling_datafile_manager.ts

-  datafile?: string,
+  logger: LoggerFacade,
+  // TODO[OASIS-6649]: Don't use object type
+  // eslint-disable-next-line  @typescript-eslint/ban-types


Were these comments left here intentionally?

Saw more instances of this comment left around but don't see the associated ticket #6649 in the OASIS JIRA Issues list for some reason.

https://optimizely.atlassian.net/browse/OASIS-6649
This should be moved to the tech debt epic. This is something thats tricky to resolve

zashraf1985

@ozayr-zaviar I have a few more change request.

Config should also export a jsonSchemaValidator just like OptimizelyOptions
isValidInstance is not logically needed on Config. We were just using config to hold this value and pass on to Optimizely constructor instead of storing it in to a local variable in the entry point. Please fix this.
Update copyright headers wherever applicable.

zashraf1985

LGTM! Thanks for cleaning up the typings mess. Great work @ozayr-zaviar

Develliot · 2022-07-01T10:25:53Z

packages/optimizely-sdk/lib/index.d.ts

-
-  export function setLogLevel(level: enums.LOG_LEVEL | string): void;
-
-  export function createInstance(config: Config): Client;


This is a breaking change as it can now be Client | null

init

4917557

ozayr-zaviar added 6 commits February 25, 2022 20:37

Event module made public

e80f0a2

typings added in shared types

5a547fc

lint fix

c58cda9

ProjectConfigManagerConfig datafile type corrected

6bb85d0

object warning suppressed

a50e87d

object parameter added in handleNewDatafile

d4b3661

ozayr-zaviar changed the title ~~feat: Typing for lite module~~ feat: Typing dependency moved to entry level modules Mar 9, 2022

ozayr-zaviar and others added 7 commits March 10, 2022 22:01

index.d.ts removed

b59f8a0

typing synced with index.d.ts

c3f628e

export types redundancy removed

abf00e5

notification center set to public

01c95eb

addNotificationListener type exported in utils package

50039ab

Merge branch 'master' into uzair/lite-typings

090e034

Notification changes

6d9982d

zashraf1985 suggested changes Mar 22, 2022

View reviewed changes

seperate client and type for lite bundles added

49a5246

zashraf1985 mentioned this pull request Mar 24, 2022

refactor: SDKOptions refactor #621

Closed

fixed some more types

331644b

zashraf1985 changed the title ~~feat: Typing dependency moved to entry level modules~~ refactor: Replaced explicit typings with auto generated ones Mar 24, 2022

zashraf1985 marked this pull request as ready for review March 24, 2022 21:08

zashraf1985 requested a review from a team as a code owner March 24, 2022 21:08

optimizely-codereview-bot added the 🤖 24 hrs label Mar 24, 2022

- removed unneeded SDKOptions interface

72430ff

- fixed some lint warnings - Added changelog

zashraf1985 approved these changes Mar 24, 2022

View reviewed changes

zashraf1985 added 2 commits March 24, 2022 16:19

trying to fix testapp

90e4149

Revert "trying to fix testapp"

b1a6e11

This reverts commit 90e4149.

zashraf1985 closed this Mar 25, 2022

zashraf1985 reopened this Mar 25, 2022

optimizely-codereview-bot assigned ozayr-zaviar Mar 25, 2022

optimizely-codereview-bot removed the 🤖 24 hrs label Mar 25, 2022

zashraf1985 unassigned ozayr-zaviar Mar 25, 2022

opti-jnguyen reviewed Mar 25, 2022

View reviewed changes

zashraf1985 suggested changes Mar 28, 2022

View reviewed changes

optimizely-codereview-bot assigned ozayr-zaviar Mar 28, 2022

comments addressed

10fcd43

zashraf1985 approved these changes Apr 7, 2022

View reviewed changes

zashraf1985 merged commit 5e8d539 into master Apr 7, 2022

zashraf1985 deleted the uzair/lite-typings branch April 7, 2022 19:30

Develliot reviewed Jul 1, 2022

View reviewed changes

opti-jnguyen mentioned this pull request Mar 31, 2023

Typescript - Breaking changes in 4.9.2 #811

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: Replaced explicit typings with auto generated ones #745

refactor: Replaced explicit typings with auto generated ones #745

ozayr-zaviar commented Feb 25, 2022 •

edited by zashraf1985

Loading

coveralls commented Feb 25, 2022 •

edited

Loading

zashraf1985 left a comment

zashraf1985 Mar 22, 2022

zashraf1985 Mar 22, 2022

opti-jnguyen Mar 25, 2022

zashraf1985 Mar 28, 2022

zashraf1985 left a comment

zashraf1985 commented Mar 24, 2022

opti-jnguyen Mar 25, 2022 •

edited

Loading

zashraf1985 Mar 25, 2022

zashraf1985 left a comment

zashraf1985 left a comment

Develliot Jul 1, 2022 •

edited

Loading


		export function setLogLevel(level: enums.LOG_LEVEL \| string): void;

		export function createInstance(config: Config): Client;

refactor: Replaced explicit typings with auto generated ones #745

refactor: Replaced explicit typings with auto generated ones #745

Conversation

ozayr-zaviar commented Feb 25, 2022 • edited by zashraf1985 Loading

Summary

Test plan

coveralls commented Feb 25, 2022 • edited Loading

zashraf1985 left a comment

Choose a reason for hiding this comment

zashraf1985 Mar 22, 2022

Choose a reason for hiding this comment

zashraf1985 Mar 22, 2022

Choose a reason for hiding this comment

opti-jnguyen Mar 25, 2022

Choose a reason for hiding this comment

zashraf1985 Mar 28, 2022

Choose a reason for hiding this comment

zashraf1985 left a comment

Choose a reason for hiding this comment

zashraf1985 commented Mar 24, 2022

opti-jnguyen Mar 25, 2022 • edited Loading

Choose a reason for hiding this comment

zashraf1985 Mar 25, 2022

Choose a reason for hiding this comment

zashraf1985 left a comment

Choose a reason for hiding this comment

zashraf1985 left a comment

Choose a reason for hiding this comment

Develliot Jul 1, 2022 • edited Loading

Choose a reason for hiding this comment

ozayr-zaviar commented Feb 25, 2022 •

edited by zashraf1985

Loading

coveralls commented Feb 25, 2022 •

edited

Loading

opti-jnguyen Mar 25, 2022 •

edited

Loading

Develliot Jul 1, 2022 •

edited

Loading