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

Update for Chai v4.0 #69

Merged
merged 1 commit into from
Nov 22, 2017
Merged

Update for Chai v4.0 #69

merged 1 commit into from
Nov 22, 2017

Conversation

meeber
Copy link
Contributor

@meeber meeber commented Dec 17, 2016

DO NOT MERGE

Hi @astorije, I'm just going through some Chai plugins to see how much the upcoming Chai v4.0 breaks things (given that it has a number of breaking changes). I noticed one breaking change in this project regarding the deep.property assertion being renamed to nested.property in Chai v4.0. Also, there's a breaking change in Chai's type-detect library regarding capitalization which impacts this plugin.

This PR does the minimum required to get chai-immutable working in Chai v4.0, but doesn't actually update the chai dev dependency since v4.0 isn't officially released yet (thus the failed Travis build.)

BREAKING CHANGE: deep.property renamed to nested.property

@astorije
Copy link
Owner

astorije commented Dec 18, 2016

Hey @meeber, thank you very much for taking the time to do this!

I must admit, I am a bit puzzled by the meaning difference between deep and nested. If I understand properly, .deep.property acts similarly to .deep.equal (i.e. deep equal a specific property of an array/object) while .nested.property will follow the current behavior of .deep.property (i.e. traverse the given path to shallowly assert a property of an array/object). Am I in the wrong?

If I am correct, then I think I will need to add test cases to match the new behavior of .deep.equal, and potentially some code as well.

Do you know what is the timeframe of v4? That will mean releasing a 2.0.0 version of chai-immutable.
Since Node.js maintenance for v0.12 ends end of this year, I was planning to release a 2.0.0 anyway, dropping support of Node.js v0.10 and v0.12 (mostly meaning removing them from Travis CI / AppVeyor and using arrow functions, let and const). But if Chai v4 is around the corner, I should wait for it to bundle both breaking upgrades together.

Regarding breaking changes, do you think chaijs/chai#744 is going to affect this plugin?


Summary of what needs to be done for Chai v4

All details at chaijs/chai#781.
I am trying to list necessary changes only. All nice additions that can be made thanks to v4 will go into GitHub issues.

@meeber
Copy link
Contributor Author

meeber commented Dec 18, 2016

You're correct about nested.property and deep.property. The new deep.property(name, val) performs deep equality comparison just like deep.equal, whereas the new nested.property allows for special syntax such as nested.property("zoo.lions[5].name", "tony") to dig down into an object/array. (Edit: they can be combined too via deep.nested.property)

One of the goals of Chai going forward is for keywords to have consistent meanings across assertions, so it's important for deep to have the same meaning when used with property as it does with equals and other assertions. Unfortunately, this means a breaking change :(

Not sure about timeframe of official release. I'm guessing mid-to-late January at the earliest. We're planning on dropping support for Node v0.10 and 0.12 as well.

Regarding #744: Ah yes, it looks like that PR might impact this plugin too (although it didn't break any tests). Basically, not.property(name, val) used to mean "assert that something has a name property but it doesn't equal val". Now, it instead means "assert that something doesn't have a name property equal to val".

@astorije
Copy link
Owner

You're correct about nested.property and deep.property. The new deep.property(name, val) performs deep equality comparison just like deep.equal, whereas the new nested.property allows for special syntax such as nested.property("zoo.lions[5].name", "tony") to dig down into an object/array. (Edit: they can be combined too via deep.nested.property)

Sounds good, and what I gathered, including the combination of both. I have created an issue to support .deep.property in this plugin after Chai v4 and this PR land: #75.

Not sure about timeframe of official release. I'm guessing mid-to-late January at the earliest. We're planning on dropping support for Node v0.10 and 0.12 as well.

Great timing for v2 of this plugin then: #74.

Regarding #744: Ah yes, it looks like that PR might impact this plugin too (although it didn't break any tests). Basically, not.property(name, val) used to mean "assert that something has a name property but it doesn't equal val". Now, it instead means "assert that something doesn't have a name property equal to val".

Well, that's bad if no tests were broken...
Tests around .not + .property are here, here, here, here, here and here.
Essentially, I do not have tests for .not + .property that test against a value and are not using .deep.
I will probably have to write tests for this prior to moving to Chai v4 as behavior will change and I need to make sure this is controlled. Essentially, I need to come up with tests that pass on the current version of Chai and break with v4.

BREAKING CHANGES:
- `.deep.property` renamed to `.nested.property`
- `.not.keys` changed to implicitly mean `.not.all.keys` instead of
  `.not.any.keys`
@meeber
Copy link
Contributor Author

meeber commented May 26, 2017

@astorije I've updated this PR for the release of Chai v4.0. As mentioned before, this does seem like a breaking change for this plugin, so I've updated the package.json accordingly.

@mlucool
Copy link

mlucool commented Jun 16, 2017

@astorije Chai 4 has been released. Is there is a plan to release this soon?

@artursvonda
Copy link

Bump! Any timeline for merging this?

@myklt
Copy link

myklt commented Jul 20, 2017

Are there any pending issues which the community could solve to release Chai v4 compatible version?

@astorije
Copy link
Owner

astorije commented Jul 27, 2017

Hey guys, sorry for this awfully long silence! I got very busy with life and other OSS projects, and didn't give enough love to chai-immutable.
I am planning to get back to this soon, to work on chai-immutable v2.0.0, which among other things will support Chai v4.

In the meantime, I understand how unacceptable it is for a library one relies on to stale like that, and I do apologize for it. To help ease the pain, I have just released v2.0.0-alpha.1 on npm, but careful, it is nothing more than master + this unreviewed PR. It is simply a temporary measure so you can get unstuck on your projects without heavily relying on me and on v2.0.0.

Does this help? Let me know if there is anything short-term I can do if necessary.

@astorije
Copy link
Owner

To anyone who needs chai-immutable to speak Chai v4, until I am done with the v2 of this plugin, you can use npm install chai-immutable@next.

@astorije
Copy link
Owner

Alright, let's make this happen. I'll merge it as-is, and if there are any issues, I'll address them later. It's been long enough, I don't have any excuses anymore 😅

@astorije
Copy link
Owner

astorije commented May 29, 2018

Hey @artursvonda and @myklt, I have just released v2.0.0-rc.2 which hopefully is the last step before full Chai v4 support (finally!). Mind giving it a shot and letting me know if it works as expected for you?

astorije added a commit that referenced this pull request Jul 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants