Skip to content
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

Add typescript support #3652

Merged
merged 17 commits into from
Jul 25, 2020
Merged

Add typescript support #3652

merged 17 commits into from
Jul 25, 2020

Conversation

GregTheGreek
Copy link
Contributor

@GregTheGreek GregTheGreek commented Jul 22, 2020

Description

This PR focuses on adding TypeScript in an incremental way. Currently allowJs is enabled allowing us to write TypeScript and JavaScript side by side. The configuration is very lenient, and in the future as we make progress on the typescript interfaces we should tighten it up.

It also:

  • Changes the build to build using tsc (future change to babel?)
  • Changes the tests to run the tsnode/register so typescript is supported
  • Adds support for up-to es8 (classes, arrow functions, etc...)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • [ x ] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • [ x ] I have selected the correct base branch.
  • [ x ] I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • [ x ] My changes generate no new warnings.
  • [ x ] Any dependent changes have been merged and published in downstream modules.
  • [ x ] I ran npm run dtslint with success and extended the tests and types if necessary.
  • [ x ] I ran npm run test:unit with success.
  • [ x ] I ran npm run test:cov and my test cases cover all the lines and branches of the added code.
  • [ x ] I ran npm run build and tested dist/web3.min.js in a browser.
  • [ x ] I have tested my code on the live network.
  • [ x ] I have checked the Deploy Preview and it looks correct.
  • [ x ] I have updated the CHANGELOG.md file in the root folder.

@GregTheGreek GregTheGreek added the 1.x 1.0 related issues label Jul 22, 2020
@GregTheGreek GregTheGreek changed the title greg/modernize Add typescript support Jul 22, 2020
@coveralls
Copy link

coveralls commented Jul 22, 2020

Coverage Status

Coverage decreased (-5.9%) to 84.321% when pulling 6f66150 on greg/modernize into b0b135c on 1.x.

spacesailor24
spacesailor24 previously approved these changes Jul 22, 2020
Copy link
Contributor

@spacesailor24 spacesailor24 left a comment

Choose a reason for hiding this comment

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

Other than the two commented imports, lgtm!

packages/web3-core/src/index.js Outdated Show resolved Hide resolved
@GregTheGreek GregTheGreek force-pushed the greg/modernize branch 2 times, most recently from fe2989e to d652438 Compare July 22, 2020 22:24
@GregTheGreek GregTheGreek marked this pull request as ready for review July 23, 2020 01:36
@GregTheGreek GregTheGreek dismissed spacesailor24’s stale review July 23, 2020 01:36

few commits since review

scripts/e2e.ganache.sh Outdated Show resolved Hide resolved
spacesailor24
spacesailor24 previously approved these changes Jul 23, 2020
@GregTheGreek
Copy link
Contributor Author

@spacesailor24 and @ryanio I updated the changelog and addressed a previous comment. Ready for re-review.

@ryanio
Copy link
Collaborator

ryanio commented Jul 24, 2020

@GregTheGreek looks good 👍 in the past we have only merged changes to dist/web3.min.js on release (like noted here), what do you think?

@GregTheGreek
Copy link
Contributor Author

@GregTheGreek looks good in the past we have only merged changes to dist/web3.min.js on release (like noted here), what do you think?

hmm let me look at that, i thought i pulled in the ones from 1.x

@GregTheGreek GregTheGreek requested a review from ryanio July 24, 2020 14:23
Copy link
Collaborator

@ryanio ryanio left a comment

Choose a reason for hiding this comment

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

LGTM!

@GregTheGreek GregTheGreek merged commit 1fc6217 into 1.x Jul 25, 2020
@GregTheGreek GregTheGreek deleted the greg/modernize branch July 26, 2020 04:14
@GregTheGreek GregTheGreek mentioned this pull request Sep 2, 2020
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.x 1.0 related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants