search

braintree / restricted-input

Restrict <input>s to certain valid characters (e.g. formatting phone or card numbers)
https://braintree.github.io/restricted-input/
MIT License
68 stars 25 forks source link

Use typescript #73

Closed crookedneighbor closed 4 years ago

crookedneighbor commented 4 years ago

Migrates package to typescript.

crookedneighbor commented 4 years ago

Okay so I reviewed this PR working from the assumption that these changes would necessitate a major version bump. I suppose it's possible that they won't, but I'm pretty sure I had a specific, concrete reason why they should and I just forgot it. Barring remembering what that was, let's go with "changing how a library is built feels like a major version level gamble."

Given the above, one change I'd like to see is converting all the faux private class methods to private or protected typescript class methods. Editors like VS Code & IDEA can automate a lot of that so it's not quite the intimidating process it could be. I wouldn't want to make those changes without a major version bump though, bc who knows what a given developer is doing digging around the prototypes. Developers are clever.

Just adding the private/protected notation to the methods is enough for me to feel fine doing a major version bump here. My main concern is that we don't introduce anything that would be a breaking change to braintree-web.

There's some other little things to tweak, but also converting forEach tests to jest loops. It's a quick process and i'd rather see the test runner handle test rates.

sure

crookedneighbor commented 4 years ago

@gesa ready for another look

gesa commented 4 years ago

how does a webdriver test fail on browser.url("http://bs-local.com:3099");

crookedneighbor commented 4 years ago

I think there are multiple builds running right now, so we may have just run out of browserstack instances running in parallel

crookedneighbor commented 4 years ago

Pausing these builds while my local builds are running.

crookedneighbor commented 4 years ago

Kicking the build off again now that my local build is done.

crookedneighbor commented 4 years ago

Yeah, I think we just need to not worry about the jsdoc stuff for now. Not a big deal for this repo, but will become very important for the braintree-web migration to typescript.