-
Notifications
You must be signed in to change notification settings - Fork 369
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
Breakup Utils Package (Breaking change) #8987
Closed
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Everything from celo/utils which used google-libphonenumber or country.js has been move here include the io.ts file.
aaronmgdr
changed the title
Aaronmgder/utils breakup
Breakup Utils Package (Breaking change)
Nov 18, 2021
…e is published since faucet must depend only on published packages
Merged
# Conflicts: # packages/attestation-service/package.json # packages/faucet/package.json # packages/sdk/CHANGELOG.md # packages/sdk/identity/package.json # packages/sdk/phone-utils/package.json # packages/sdk/utils/package.json # packages/sdk/utils/src/io.ts # yarn.lock
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR separates our the Utils package into files that depend on
google-libphonenumber
andcountry-data
. and those that do not. Our goal here is to reduce the package size of@celo/utils
as many will want to use the utils but wont need any phone number capability.Other changes
"exclude": ["**/*.test.ts"]
to tsconfig for the @celo/utils and phone-utils packages because before the lib folder had all our tests in it which was then published to NPM. which i think would increase the package size.Tested
Ran the original tests apart from some import changes all is good
Related issues
Backwards compatibility
No. anyone who was importing
{PhoneNumberUtils, getCountryEmoji, getPhoneHash, LocalizedCountry, Countries, validatePhone, validateInput } from "@celo/utils"
must now import from@celo/phone-utils
Furthermore the
io.ts
file was split up andE164PhoneNumberType, AttestationServiceStatusResponseType, AttestationServiceTestRequestType, AttestationRequestType, GetAttestationRequestType, AttestationRequest, AttestationResponseType, AttestationResponse
must be imported from@celo/phone-utils
too.This is because all those types indirectly depend on
E164PhoneNumberType
which usesisE164NumberStrict
. One alternative is to change this to useisE164Number
from celo/base which is regeex based instead but im not sure of the affects of that. but it would greatly simplify a lot here.Documentation
generated some doc files. not sure if something else is needed