Spike: Camel case syntax #784

papandreou · 2020-12-23T10:54:02Z

This is a super inefficient proof-of-concept implementation so that we can discuss the syntax etc.

If we can achieve some kind of consensus, I promise to clean it up and optimize it :)

Work through edge cases discovered by auto-converting the test suite to camel case: 74903a4
Think about how we might capture the full context in middle-rocket cases like expect([1, 2,3]).whenPassedAsParametersTo(Math.max).toEqual(4) -- right now that just fails with expected 3 to equal 4 here.
Think about flag forwarding again (https://gitter.im/unexpectedjs/unexpected?at=5ce1417463dea422b4abaa6c)
Figure out what expect.it(fn) should map to.

alexjeffburke · 2021-01-04T14:08:30Z

I took a look at this PR - some thoughts follow.

Implementation wise I think it is very straightforward - one thing I learnt from trying unassessed was that we may may not want to expose all the assertions like this. In it I had somewhat of a whitelist, although it was implemented by looking at the assertion string / type information and at least filtering out any that were middle-rocket.

On a perhaps more controversial point, I can't help but be in two minds about whether this should really be in unexpected core. It almost feels like it's might be something else if we expose the methods like this. From what I can tell, we can almost do this by making use of being able to hook unexpected - perhaps we should consider making that infrastructure work and putting this out under a new name?

alexjeffburke · 2021-01-04T14:09:44Z

lib/createTopLevelExpect.js

  if (typeof format === 'undefined') {
    return this._outputFormat;
  } else {
+    this._assertTopLevelExpect();


IIRC I hit an issue with this myself at some point and I think this fix may want to be applied directly to master.

Yeah, I agree: 5349b0fa

alexjeffburke · 2021-01-04T14:12:53Z

lib/createTopLevelExpect.js

+  if (args.length < 1) {
+    throw new Error('The expect function requires at least one parameter.');
+  } else if (args.length === 1) {
+    return this._camelCaser(context, this.findTypeOf(subject), subject);


Re the latter part of my comment here, it's weakening the two parameter rule and making a single argument suddenly make the library behave very differently that got me pondering whether this bends things too far.

I don't think it's too bad -- it actually aligns the one parameter expect case with what the nested expect made available to assertion rule handlers does:

unexpected/lib/createTopLevelExpect.js

Line 1313 in 55268ea

throw new Error('The expect function requires at least one parameter.');

... And if we agree about a long term goal of deprecating/removing the old string-based syntax, I think we can live with a bit of temporary overloading.

papandreou · 2021-01-04T16:52:34Z

Implementation wise I think it is very straightforward - one thing I learnt from trying unassessed was that we may may not want to expose all the assertions like this. In it I had somewhat of a whitelist, although it was implemented by looking at the assertion string / type information and at least filtering out any that were middle-rocket.

This implementation preserves middle rocket support via this syntax:

expect([1, 2, 3]).whenPassedAsParametersTo(Math.max).toEqual(3);

Then we can have a separate conversation about whether to unsupport middle rocket altogether. Last time we talked about it, I remember that we were a bit back and forth about whether to part with it, and that especially unexpected-dom had some very attractive use cases for it.

We've also talked about requiring middle rocket assertions to declare the type of what they're expect.shifting, which has the potential for us to produce more precise typings. I would prefer to have a separate discussion about that, though.

On a perhaps more controversial point, I can't help but be in two minds about whether this should really be in unexpected core. It almost feels like it's might be something else if we expose the methods like this. From what I can tell, we can almost do this by making use of being able to hook unexpected

In my opinion the camel case syntax is superior to what we have now (and less controversial), so I think we should go all in on it, switch all the docs over to use it, and aim to remove the current assertion syntax. If we hook it in and keep it in a plugin, we'll be sending mixed signals and end up with a fragmented ecosystem.

perhaps we should consider making that infrastructure work and putting this out under a new name?

That's called unassessed, isn't it? 😼

In my opinion we've essentially already done that and proven that it's pleasant to work with, so I think now's the time to put it in core.

…expect #784 (comment)

alexjeffburke · 2021-01-04T17:52:04Z

test/camelCaseSyntax.spec.js

+      it('should fail', () => {
+        expect(() =>
+          expect({ foo: 123 }).toSatisfy({
+            foo: expect.toBeANumber().andToBeGreaterThan(200),


I wonder if we could still attach these to a .it object and simply hand one out that has had the camelCaser run over it when we're in that mode?

Reason this occurs to me is that I think the .it. still serves a purpose syntactically in saying "and use the subject from the situation in which you are called.

Yeah, I was a bit back and forth on that, decided that it looked fairly clean without any it. Could you spell out with some examples exactly which alternative(s) you're thinking about? expect.it.toBeANumber()? To me that reminded me too much of chai's weirdness where the dot notation and camel case are mixed -- expect(foo).to.shallowDeepEqual(bar) 🙀

Hmm I see what you're getting at. Let me sit with this some more...

More thinking aloud, for the purposes of discussion, what do we do with the value returning form of expect.it? Perhaps the normal assertions should be callable directly, but we provide a .it() that is specifically meant to be used only as .it(value => { ... }) ?

Hmm, yeah, the expect.it(fn) case is interesting to consider. It shouldn't really be a problem to keep supporting it as-is.

You could argue that if we're going to migrate it over to something that's not backwards compatible (or if we want to add another variant of it that's easier on the eyes in a camel case context), we might as well switch to something completely different that isn't necessarily an overloading of the expect.it replacement.

alexjeffburke · 2021-01-04T17:54:17Z

test/camelCaseSyntax.spec.js

+        ).toThrow(
+          'expected { foo: 123 } to satisfy\n' +
+            '{\n' +
+            "  foo: expect.it('to be a number')\n" +


As we proceed to making this user-ready we probably need to consider what to do here - perhaps we could switch out the expect.it type such that this would render the camel cases names? Wondering if that can fit with my comment about about keeping the .it. above as well.

Yeah, absolutely, that would be fairly easy. We can try to make the rendering respect the syntax that was actually used in the source code, or just make an unconditional switch to camel case.

alexjeffburke · 2021-01-04T17:56:53Z

test/camelCaseSyntax.spec.js

+    describe('and extra chaining with .or...', () => {
+      it('should succeed', () => {
+        expect({ foo: 123 }).toSatisfy({
+          foo: expect.toBeAString().orToBeGreaterThan(100),


Im not too keen of having different spelling of the assertions like this - I think that would get confusing when it came to documentation and the like.

Thinking aloud were ok chaining through a .or. we could potentially generate the object with all these assertions only once.

That's fair, we have a bunch of wiggle room wrt. the syntax here. Again, I'm not too keen on mixing dot notation and camel case, but something like expect.toBeAString().or().toBeGreaterThan(100) could be arranged?

That is far better to my eyes at least!

Removes support for expect(oneArg) returning an object with .and etc., but we can maybe get that back later

…rocket assertion

pingvinen · 2022-09-08T08:09:58Z

In my opinion the camel case syntax is superior to what we have now (and less controversial)

In my opinion the current assertion syntax is far superior to this DSL-chaining thing. It is one of the primary reasons why I use unexpected in the first place. If unexpected is used in the "exact" same way as the rest of the assertion frameworks, then why do we need unexpected at all?

papandreou · 2022-09-08T08:40:37Z

Hopefully because of the extensibility and quality of the output when an assertion fails? :)

papandreou self-assigned this Dec 23, 2020

papandreou added the Major label Jan 3, 2021

alexjeffburke reviewed Jan 4, 2021

View reviewed changes

papandreou added a commit that referenced this pull request Jan 4, 2021

Don't prohibit expect.outputFormat() without a parameter on a nested …

5349b0f

…expect #784 (comment)

alexjeffburke reviewed Jan 4, 2021

View reviewed changes

papandreou mentioned this pull request Jan 4, 2021

Add the parent expect to the prototype chain of a child expect #785

Merged

papandreou force-pushed the feature/camelCase branch from c77267f to 7eb88b9 Compare April 25, 2022 19:43

papandreou changed the base branch from master to feature/childInheritsFromParentExpect April 25, 2022 19:44

papandreou force-pushed the feature/childInheritsFromParentExpect branch from 23105f0 to 16e4b28 Compare April 25, 2022 19:46

papandreou force-pushed the feature/camelCase branch from 7eb88b9 to c70d2d5 Compare April 25, 2022 19:47

Base automatically changed from feature/childInheritsFromParentExpect to master April 26, 2022 18:45

papandreou force-pushed the feature/camelCase branch from c70d2d5 to 66eac00 Compare April 26, 2022 21:37

papandreou force-pushed the feature/camelCase branch from 66eac00 to 7b0eb42 Compare May 28, 2022 22:20

papandreou added 12 commits May 31, 2022 07:42

Sketch out some basic tests

5b126fe

Slow, naive implementation of the basic cases

f59f4e3

Support camel case syntax within an assertion

b211873

Removes support for expect(oneArg) returning an object with .and etc., but we can maybe get that back later

Add expect.it shorthands

5a1d6c1

Fix additional promise methods and expect.it

933f822

Remove no longer relevant test

ff045ca

wrapped expect: Back to just using addAdditionalPromiseMethods

9836925

Expect the full context to be captured when delegating from a middle …

2bcf9a4

…rocket assertion

WIP

9aea0a4

Fiddle

d713df1

Fix shifty legacy mode

0ac7ffc

fix

553182f

papandreou added 7 commits May 31, 2022 07:42

Hacketyfix

13996cf

fix

6c72280

Unhack

638162c

simplify

060a56e

Clean

f96b386

simplify

834abfe

clean

a3b3ee0

papandreou force-pushed the feature/camelCase branch from ed68984 to a3b3ee0 Compare May 31, 2022 05:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Spike: Camel case syntax #784

Spike: Camel case syntax #784

papandreou commented Dec 23, 2020 •

edited

Loading

alexjeffburke commented Jan 4, 2021

alexjeffburke Jan 4, 2021

papandreou Jan 4, 2021

alexjeffburke Jan 4, 2021 •

edited

Loading

papandreou Jan 4, 2021 •

edited

Loading

papandreou commented Jan 4, 2021

alexjeffburke Jan 4, 2021

papandreou Jan 4, 2021

alexjeffburke Jan 4, 2021

papandreou Jan 4, 2021 •

edited

Loading

alexjeffburke Jan 4, 2021

papandreou Jan 4, 2021

alexjeffburke Jan 4, 2021

papandreou Jan 4, 2021

alexjeffburke Jan 4, 2021

pingvinen commented Sep 8, 2022

papandreou commented Sep 8, 2022

Spike: Camel case syntax #784

Are you sure you want to change the base?

Spike: Camel case syntax #784

Conversation

papandreou commented Dec 23, 2020 • edited Loading

alexjeffburke commented Jan 4, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexjeffburke Jan 4, 2021 • edited Loading

Choose a reason for hiding this comment

papandreou Jan 4, 2021 • edited Loading

Choose a reason for hiding this comment

papandreou commented Jan 4, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

papandreou Jan 4, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pingvinen commented Sep 8, 2022

papandreou commented Sep 8, 2022

papandreou commented Dec 23, 2020 •

edited

Loading

alexjeffburke Jan 4, 2021 •

edited

Loading

papandreou Jan 4, 2021 •

edited

Loading

papandreou Jan 4, 2021 •

edited

Loading