-
Notifications
You must be signed in to change notification settings - Fork 71
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
Update all dependencies to latest #42
base: master
Are you sure you want to change the base?
Conversation
… dependency. The jest types where conflicting with mocha types. There were no references in the project to jest.
Addresses changed property name and update to record with immutable fields.
Typescript 4 NullKeyword is now a subtype of the LiteralType
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
@googlebot I signed it! |
From a cursory glance this looks great! I'll scrutinize a little deeper in the coming week. For better and worse, I rely solely on the unit tests rather than insights from real-world usage due to its still limited prevalence in the wild. All that to say, I may write a few new unit tests to see if there's anything this might have missed with changes in TS 4. This leads me to conjecture that it might be time to make this a major version release. For the most part, the tool has been stable in terms of new PRs, after all it's been over a year since its launch. Some new features get added and new language constructs are handled, but by and large this feels fairly concrete. A major version release will provide the greatest benefit in changes such as this PR where the underlying TS target is upgraded. What are your thoughts regarding this 1.0.0 major version idea? |
Based on the possibility that the dependency changes might represent a breaking change for some, the major version idea makes a lot of sense. As far as a 1.0.0 release, in my very limited exploration (I just found this project yesterday), I have seen a few rough edges (possibly user errors) that might ideally be working in a 1.0 release. The most notably problems:
An aside,
|
I added a issue for the first case I mentioned above, #43. A correction regarding the second case above, it seems like I may have encountered two cases. Most of the cases I observed were actually cases where the object was declared in another file, like the first issue. The other case was when a |
…r and mini-css-extract-plugin
@jasco @ryanmcdermott Is anyone going t merge this? Also is the issue with empty objects fixed? This library seems promising but right now the types are not generated correctly - in most cases - so it is also useless. I could maybe help if you create an issue and describe at least roughly where the problem is. 🙂 |
Any update now?
|
This updates all the project dependencies to latest, including porting to Typescript 4.
All the tests and ci pass.
When reviewing, note in particular the destructuring immutable copy that was added in intermock.ts at line 334. The Typescript fields are now declared readonly so this somewhat complicated casting and destructuring approach, which basically should mirror what was there previously, was used to do a deep copy-update.