Skip to content

build: Introduce TypeScript into optimizely-sdk build #504

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

Merged
merged 28 commits into from
Jul 31, 2020

Conversation

mjc1283
Copy link
Contributor

@mjc1283 mjc1283 commented Jun 11, 2020

Summary

Add TypeScript into the build for optimizely-sdk. The goal is to enable future development with TS and incremental conversion of modules from JS to TS.

  • Uses the same TS version as subpackages (datafile-manager, event-processor, etc.)
  • Uses rollup-plugin-typescript2 to add TS compilation step to rollup build
  • tsconfig.json for optimizely-sdk is based on the share root tsconfig.json, with appropriate overrides
  • Lint implementation is updated for TS compatibility, based on React SDK's
  • Mocha test suite is updated for TS compatibility: types installed, command updated to use ts-node, based on official Mocha TS example
  • Configuration for Karma-based tests is updated to use ts-loader
  • Converted string_value_validator and several other smaller modules as examples and litmus tests of the build changes

Test plan

  • Manual testing using local build and linked Node app
  • Existing tests must pass - there are no intended functional changes
  • Inspected and compared build output from master branch, and this branch

@mjc1283 mjc1283 added the WIP label Jun 11, 2020
@mjc1283 mjc1283 self-assigned this Jun 11, 2020
@coveralls
Copy link

coveralls commented Jun 11, 2020

Coverage Status

Coverage increased (+0.07%) to 96.771% when pulling 8b55526 on mcarroll/optimizely-sdk-typescript into 0427cdd on master.

@mjc1283 mjc1283 removed the WIP label Jun 16, 2020
@mjc1283 mjc1283 marked this pull request as ready for review June 16, 2020 20:19
@mjc1283 mjc1283 requested a review from a team as a code owner June 16, 2020 20:19
@mjc1283 mjc1283 removed their assignment Jun 16, 2020
Copy link
Contributor

@mikeproeng37 mikeproeng37 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, though the browser tests are failing: https://travis-ci.org/github/optimizely/javascript-sdk/jobs/699104903

@mjc1283
Copy link
Contributor Author

mjc1283 commented Jun 16, 2020

Oops, looks like I will have to address the Karma tests in this PR.

@mjc1283 mjc1283 marked this pull request as draft June 16, 2020 23:16
@mjc1283 mjc1283 self-assigned this Jun 16, 2020
@mjc1283 mjc1283 added the WIP label Jun 16, 2020
@mjc1283 mjc1283 removed the WIP label Jun 17, 2020
@mjc1283 mjc1283 removed their assignment Jun 17, 2020
@mjc1283 mjc1283 marked this pull request as ready for review June 17, 2020 22:35
Copy link
Contributor

@mikeproeng37 mikeproeng37 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did another pass, still lgtm

"sinon": "^2.3.1",
"ts-node": "^8.10.2",
"typescript": "^3.3.3333",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks wrong. Is this intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the SDK packages are using this version right now - I figured we should keep them all on the same version and upgrade in sync.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But 3.3.3333 doesnt appear to be a valid release for this package.

Oops, I was wrong. https://github.com/microsoft/TypeScript/releases/tag/v3.3.3333

Still a strange version to pick, but it is in fact valid.

Matt Carroll and others added 9 commits July 14, 2020 10:13
Summary:

Convert event_tags_validator module from JS to TS
Revise user_profile_service_validator module

Test plan:

Existing unit tests
… to TS (#524)

* Convert project_config_schema and json_schema_validator modules to TS

* Address Mike's comments

* Update validate function to accept empty object
Summary:

Convert attributes_validator module from JS to TS.

Test plan:

Existing unit tests
* Convert event_tag_utils module to TS

* Convert event_tag_utils module to TS

* Add type guard to avoid using any

* Move 'if (eventTags)' to top level functions

* Move let variables to inside the functions
@mjc1283 mjc1283 changed the title feat: Introduce TypeScript into optimizely-sdk build build: Introduce TypeScript into optimizely-sdk build Jul 31, 2020
@mjc1283 mjc1283 merged commit d00751b into master Jul 31, 2020
@mjc1283 mjc1283 deleted the mcarroll/optimizely-sdk-typescript branch July 31, 2020 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants