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

Consuming SerenityJS API from es5/6 #22

Closed
InvictusMB opened this Issue Feb 4, 2017 · 10 comments

Comments

2 participants
@InvictusMB
Contributor

InvictusMB commented Feb 4, 2017

Currently SerenityJS kind of forces you to use TypeScript which is not always feasible. It would be nice to expose an API that can be consumed from es5/6 code without an extra boilerplate.

There are two things that make me sad:

  • Step decorators require boilerplate to consume them in es5.
  • Defining a Task requires writing a class. Having a class only to host one performAs method is too Java like redundant.

The following steps could address those issues:

  • Expose a helper that will decorate a Task Provide a task description via toString method instead of a decorator
  • Make function a first-class Performable (#21)

I have converted getting started tutorial to es6 with Babel as a proof of concept. It required me to create a helper to reduce a boilerplate and wrap plain functions into SerenityJS compatible Task.

Task definitions ended up like this:

export const AddATodoItem = createTask('{0} adds a Todo Item called #itemName',
  {called: 'itemName'},
  function(actor) {
    return actor.attemptsTo(
      Enter.theValue(this.itemName)
        .into(TodoList.What_Needs_To_Be_Done)
        .thenHit(Key.ENTER)
    );
  }
);

Which was still far from perfect but a good place to start if you are limited to es5/6.

@jan-molak

This comment has been minimized.

Show comment
Hide comment
@jan-molak

jan-molak Feb 4, 2017

Owner

Interesting idea, thanks!

Could you please share what factors prevent you from using TypeScript (via ts-node)?

Owner

jan-molak commented Feb 4, 2017

Interesting idea, thanks!

Could you please share what factors prevent you from using TypeScript (via ts-node)?

@InvictusMB

This comment has been minimized.

Show comment
Hide comment
@InvictusMB

InvictusMB Feb 4, 2017

Contributor

@jan-molak
Namely introducing TypeScript to code base is what we want to avoid. We have a preference and are making an investment into es6 + flow.

Contributor

InvictusMB commented Feb 4, 2017

@jan-molak
Namely introducing TypeScript to code base is what we want to avoid. We have a preference and are making an investment into es6 + flow.

jan-molak added a commit that referenced this issue Feb 12, 2017

refactor(core): Domain classes represent the domain concepts better
- `Performable` is really an `Activity` that an `Actor` can perform, therefore it should be called as such.
- What used to be an `Activity` is now a `RecordedActivity`. `RecordedActivity` is just a "tiny type", that captures the name of the real `Activity` and works alongside with `Outcome` to capture the result and other meta data related to the performance.
- To make the model consistent, `Scene` is now a `RecordedScene`
- To help distinguish `RecordedActivity` related to a `Task` and `Interaction`, the `@step()` annotation can now take an optional second arugment `ActivityType.Task` or `ActivityType.Interaction`. A `@step` that doesn't specify its type is considerd to be of type `Task` by default as 90% of time you'll be writing Tasks not Interactions.
- The distinction between a `RecordedTask` and `RecordedInteraction` will help to make the configuration of the `Photographer` more fine-grained, so that we could for example tell it to only capture screenshots of interactions rather than all the tasks. This will also help to enable the granularity needed for #18.
- `See` and `CompareNotes` are now `Interactions` rather than generic `Activities` (pretending to be `Tasks`), as performing an assertion is also an interaction with the system. No need for special treatment of those two classes.
- The `Interaction` interface is also corrected to allow for `Actors` that `UseAbilities` but also `AnswerQuestions`

Those changes should not affect the consumers of the Serenity/JS APIs and will help tackle the tech debt before it spreads ;-)

Enables #18, #22, #23

jan-molak added a commit that referenced this issue Apr 26, 2017

feat(core): support for ES6-style task definitions
affects: @serenity-js/cucumber-2, serenity-js

- Instead of using the @step annotation it's enough for a task to define a toString() method, which will be used to determine its description #22
- RecordedTask and RecordedActivity are now replaced by RecordedActivity, which can also define the location where it was invoked to support #18
- A more functional-style DSL provided to make prototyping faster, for example, to create a task it's enough to write:  task_where('{0} proceeds to checkout') #21

ISSUES CLOSED: #21, #22
@jan-molak

This comment has been minimized.

Show comment
Hide comment
@jan-molak

jan-molak Apr 26, 2017

Owner

@InvictusMB,

Does this implementation solve the problem?

You should be able to use either the ES6-style implementation of a task:

class Follow {

    static the = (personOfInterest: string) => new Follow(personOfInterest);

    performAs(actor: PerformsTasks): PromiseLike<void> {
        // ...
    }

    toString = () => `{0} follows the ${ this.personOfInterest }`;

    constructor(personOfInterest) {
        this.personOfInterest = personOfInterest;
    }
}

which can be passed to the actor to perform:

alice.attemptsTo(Follow.the('white rabbit'))

Or alternatively, you can define the task as follows:

const follow_the = (person_of_interest: string) => task_where(`{0} follows the ${person_of_interest}`);

and then:

alice.attemptsTo(follow_the('white rabbit'))

which is closer to the functional interface we talked about in #21 as it allows for tasks to be composed as well:

const follow_the = (person_of_interest: string) => task_where(`{0} follows the ${person_of_interest}`, 
  task_where('{0} notices the rabbit'),
  task_where('{0} decides to follow the rabbit'),
  // ... and so on. You can also have Click.on(...) and other tasks and interactions here.
);

Looking forward to hearing your thoughts.
Jan

Owner

jan-molak commented Apr 26, 2017

@InvictusMB,

Does this implementation solve the problem?

You should be able to use either the ES6-style implementation of a task:

class Follow {

    static the = (personOfInterest: string) => new Follow(personOfInterest);

    performAs(actor: PerformsTasks): PromiseLike<void> {
        // ...
    }

    toString = () => `{0} follows the ${ this.personOfInterest }`;

    constructor(personOfInterest) {
        this.personOfInterest = personOfInterest;
    }
}

which can be passed to the actor to perform:

alice.attemptsTo(Follow.the('white rabbit'))

Or alternatively, you can define the task as follows:

const follow_the = (person_of_interest: string) => task_where(`{0} follows the ${person_of_interest}`);

and then:

alice.attemptsTo(follow_the('white rabbit'))

which is closer to the functional interface we talked about in #21 as it allows for tasks to be composed as well:

const follow_the = (person_of_interest: string) => task_where(`{0} follows the ${person_of_interest}`, 
  task_where('{0} notices the rabbit'),
  task_where('{0} decides to follow the rabbit'),
  // ... and so on. You can also have Click.on(...) and other tasks and interactions here.
);

Looking forward to hearing your thoughts.
Jan

@InvictusMB

This comment has been minimized.

Show comment
Hide comment
@InvictusMB

InvictusMB Apr 28, 2017

Contributor

@jan-molak
Makes sense to me. Although I have mixed feelings about underscored naming convention in tests. I would prefer to have DSL-like APIs everywhere and not to care if a particular task is a function or implements Task interface. As that is an implementation detail and may be subject to change.
So I would emphasize that functional and class implementations can be used interchangeably.
As going from

class Follow {

    static the = (personOfInterest: string) => new Follow(personOfInterest);

    performAs(actor: PerformsTasks): PromiseLike<void> {
        // ...
    }

    toString = () => `{0} follows the ${ this.personOfInterest }`;

    constructor(personOfInterest) {
        this.personOfInterest = personOfInterest;
    }
}

to

const Follow = {
  the(personOfInterest : string) {
    return task_where(`{0} follows the ${personOfInterest }`);
  }
}

wouldn't require any change in the consuming code.

Also I believe #actor would look better than {0} as a placeholder in templates.

Contributor

InvictusMB commented Apr 28, 2017

@jan-molak
Makes sense to me. Although I have mixed feelings about underscored naming convention in tests. I would prefer to have DSL-like APIs everywhere and not to care if a particular task is a function or implements Task interface. As that is an implementation detail and may be subject to change.
So I would emphasize that functional and class implementations can be used interchangeably.
As going from

class Follow {

    static the = (personOfInterest: string) => new Follow(personOfInterest);

    performAs(actor: PerformsTasks): PromiseLike<void> {
        // ...
    }

    toString = () => `{0} follows the ${ this.personOfInterest }`;

    constructor(personOfInterest) {
        this.personOfInterest = personOfInterest;
    }
}

to

const Follow = {
  the(personOfInterest : string) {
    return task_where(`{0} follows the ${personOfInterest }`);
  }
}

wouldn't require any change in the consuming code.

Also I believe #actor would look better than {0} as a placeholder in templates.

jan-molak added a commit that referenced this issue Apr 28, 2017

fix(core): both the @step and Activity::toString can use an #actor to…
…ken instead of {0}

affects: serenity-js

See @InvictusMB suggestion in #22
@jan-molak

This comment has been minimized.

Show comment
Hide comment
@jan-molak

jan-molak Apr 28, 2017

Owner

Thanks for your feedback!

I like the suggestion regarding the #actor; The following syntax should work fine and is definitely much more compact than the original:

const Follow = {
  the: (personOfInterest : string) => task_where(`#actor follows the ${ personOfInterest }`);
}

Does this solve the original issue of being able to use Serenity/JS with Flow?

Owner

jan-molak commented Apr 28, 2017

Thanks for your feedback!

I like the suggestion regarding the #actor; The following syntax should work fine and is definitely much more compact than the original:

const Follow = {
  the: (personOfInterest : string) => task_where(`#actor follows the ${ personOfInterest }`);
}

Does this solve the original issue of being able to use Serenity/JS with Flow?

@InvictusMB

This comment has been minimized.

Show comment
Hide comment
@InvictusMB

InvictusMB Apr 29, 2017

Contributor

@jan-molak
Yes. After using toString in addition to decorators the rest becomes a matter of preference of functional or class-based style.

I also meant task_where and describe_as with regard to APIs in previous reply. Snake case just feels a bit unusual naming convention for JS. That's not a big deal to me as I could still have my own wrappers to use Task.where() and describe.as() or TaskWhere() and describeAs() if I wish to stick to camel case convention. But in the long run this may influence SerenityJS adoption.

Just for reference:
Some stats

Airbnb naming convention
JavaScript Standard Style
Google naming convention
Crockford style

I don't know any widely used JS style guides with snake case naming but there is at least one research claiming that it's better for readability in some cases.

Contributor

InvictusMB commented Apr 29, 2017

@jan-molak
Yes. After using toString in addition to decorators the rest becomes a matter of preference of functional or class-based style.

I also meant task_where and describe_as with regard to APIs in previous reply. Snake case just feels a bit unusual naming convention for JS. That's not a big deal to me as I could still have my own wrappers to use Task.where() and describe.as() or TaskWhere() and describeAs() if I wish to stick to camel case convention. But in the long run this may influence SerenityJS adoption.

Just for reference:
Some stats

Airbnb naming convention
JavaScript Standard Style
Google naming convention
Crockford style

I don't know any widely used JS style guides with snake case naming but there is at least one research claiming that it's better for readability in some cases.

@InvictusMB InvictusMB closed this Apr 29, 2017

@jan-molak

This comment has been minimized.

Show comment
Hide comment
@jan-molak

jan-molak Apr 29, 2017

Owner

Yeah, that's a good point; My thinking around it was that task_where (and soon interaction_where, question_about etc) is physically close to the textual description of the task, so the snake_case notation makes it easier to read as a sentence:

task_where(`#actor books a ticket to from ${ origin } to ${ destination }`)

vs

taskWhere(`#actor books a ticket to from ${ origin } to ${ destination }`)

Having said that, those functions would be used with camelCased tasks, which might (?) make it look a bit awkward:

task_where(`#actor books a ticket to from ${ origin } to ${ destination }`,
  SelectOriginAirport.of(origin),
  SelectDestinationAirport.of(destination),
  // ...
)

A nice way to do it would be to have a static where on the Task - Task.where as per your example, but to my knowledge TypeScript doesn't allow static methods on interfaces.

Owner

jan-molak commented Apr 29, 2017

Yeah, that's a good point; My thinking around it was that task_where (and soon interaction_where, question_about etc) is physically close to the textual description of the task, so the snake_case notation makes it easier to read as a sentence:

task_where(`#actor books a ticket to from ${ origin } to ${ destination }`)

vs

taskWhere(`#actor books a ticket to from ${ origin } to ${ destination }`)

Having said that, those functions would be used with camelCased tasks, which might (?) make it look a bit awkward:

task_where(`#actor books a ticket to from ${ origin } to ${ destination }`,
  SelectOriginAirport.of(origin),
  SelectDestinationAirport.of(destination),
  // ...
)

A nice way to do it would be to have a static where on the Task - Task.where as per your example, but to my knowledge TypeScript doesn't allow static methods on interfaces.

@InvictusMB

This comment has been minimized.

Show comment
Hide comment
@InvictusMB

InvictusMB Apr 29, 2017

Contributor

A nice way to do it would be to have a static where on the Task - Task.where as per your example, but to my knowledge TypeScript doesn't allow static methods on interfaces.

This is an interesting topic. My first guesses were 2 ways that I would go in C#. Namely:

  1. Rename Task interface to ITask and make Task a collection of utilites such as static Task.where():ITask.
  2. Convert Task to an abstract class and then extract ITask interface from it.

But apparently I prefix is discouraged in TS while in C# it was used mostly for historical reasons (like Microsoft legacy from COM era).

However, the answer is much simpler. In TS a class can be used anywhere an interface can be. So just just converting Task interface to an abstract class should be sufficient.

export abstract class Task extends Activity {
  abstract performAs(actor: PerformsTasks): PromiseLike<void>;

  static where(description: string, ...activities: Activity[]): Task {
    return new AnonymousTask(description, activities);
  }
}

And then I had another round on this "interface vs abstract type" question but in context of TS. And so far I see no reason to use interface in TS at all.
Because both class and interface definitions ultimately produce a type definition. But then why not use class when you want taxonomy and inheritance and type when you only need type checking?
This entry seems to be somewhat outdated. I've found no difference with regard to type checking in behavior of interface FooBarBaz extends Foo, Bar, Baz { } compared to type FooBarBaz = Foo & Bar & Baz except for error messages which report actual intersection instead of FooBarBaz type alias. See example.

There are similiar concerns raised for Flow typing here and there. Which makes me believe that the "interface" concept in TS is only a legacy from flawed type systems in .Net and Java worlds where it was the only way to express a contract. But it seems redundant when you have a powerful type system like TypeScript or Flow.

Contributor

InvictusMB commented Apr 29, 2017

A nice way to do it would be to have a static where on the Task - Task.where as per your example, but to my knowledge TypeScript doesn't allow static methods on interfaces.

This is an interesting topic. My first guesses were 2 ways that I would go in C#. Namely:

  1. Rename Task interface to ITask and make Task a collection of utilites such as static Task.where():ITask.
  2. Convert Task to an abstract class and then extract ITask interface from it.

But apparently I prefix is discouraged in TS while in C# it was used mostly for historical reasons (like Microsoft legacy from COM era).

However, the answer is much simpler. In TS a class can be used anywhere an interface can be. So just just converting Task interface to an abstract class should be sufficient.

export abstract class Task extends Activity {
  abstract performAs(actor: PerformsTasks): PromiseLike<void>;

  static where(description: string, ...activities: Activity[]): Task {
    return new AnonymousTask(description, activities);
  }
}

And then I had another round on this "interface vs abstract type" question but in context of TS. And so far I see no reason to use interface in TS at all.
Because both class and interface definitions ultimately produce a type definition. But then why not use class when you want taxonomy and inheritance and type when you only need type checking?
This entry seems to be somewhat outdated. I've found no difference with regard to type checking in behavior of interface FooBarBaz extends Foo, Bar, Baz { } compared to type FooBarBaz = Foo & Bar & Baz except for error messages which report actual intersection instead of FooBarBaz type alias. See example.

There are similiar concerns raised for Flow typing here and there. Which makes me believe that the "interface" concept in TS is only a legacy from flawed type systems in .Net and Java worlds where it was the only way to express a contract. But it seems redundant when you have a powerful type system like TypeScript or Flow.

@jan-molak

This comment has been minimized.

Show comment
Hide comment
@jan-molak

jan-molak Apr 29, 2017

Owner

Intriguing, thanks for researching this topic so thoroughly!

So it seems like a TypeScript class can extend an abstract class, as well as implement it...

I wonder if there could be any unwanted side effects of converting screenplay interfaces (Task/Interaction/Question) to abstract classes? 🤔

If an abstract class is identical to an interface as far as the type definition is concerned, no client code would need to change.

Owner

jan-molak commented Apr 29, 2017

Intriguing, thanks for researching this topic so thoroughly!

So it seems like a TypeScript class can extend an abstract class, as well as implement it...

I wonder if there could be any unwanted side effects of converting screenplay interfaces (Task/Interaction/Question) to abstract classes? 🤔

If an abstract class is identical to an interface as far as the type definition is concerned, no client code would need to change.

@InvictusMB

This comment has been minimized.

Show comment
Hide comment
@InvictusMB

InvictusMB Apr 30, 2017

Contributor

I would expect no side effects from this change. At least as long as the latest TypeScript versions are used. The only difference is that types and therefore interfaces leave no traces after compilation while abstract class always produces some backing code.

This shows some really curious semantics in TS.
With interfaces extends literally means extension as an algebraic sum of base type and extension type.
At the same time implements with classes also creates a new interface type definition which effectively extends base class interface. So it works the same way as extends for interfaces.
While extends actually means inheriting implementation in addition to what implements does.

Also there is one gotcha. Imagine this example:

abstract class Foo {
    abstract foo(): void;
}

interface Bar extends Foo {
    bar(): void;
}

class Baz implements Bar {
    foo() { }
    bar() { }
}

So far everything is fine and the code compiles. But if you add a private member to abstract class Foo, class Baz will no longer satisfy an interface. Unless it explicitly inherits Foo. And Baz is not allowed to have its own private member of the same name.

The fact that private members can influence a class interface type makes absolutely no sense to me.

Contributor

InvictusMB commented Apr 30, 2017

I would expect no side effects from this change. At least as long as the latest TypeScript versions are used. The only difference is that types and therefore interfaces leave no traces after compilation while abstract class always produces some backing code.

This shows some really curious semantics in TS.
With interfaces extends literally means extension as an algebraic sum of base type and extension type.
At the same time implements with classes also creates a new interface type definition which effectively extends base class interface. So it works the same way as extends for interfaces.
While extends actually means inheriting implementation in addition to what implements does.

Also there is one gotcha. Imagine this example:

abstract class Foo {
    abstract foo(): void;
}

interface Bar extends Foo {
    bar(): void;
}

class Baz implements Bar {
    foo() { }
    bar() { }
}

So far everything is fine and the code compiles. But if you add a private member to abstract class Foo, class Baz will no longer satisfy an interface. Unless it explicitly inherits Foo. And Baz is not allowed to have its own private member of the same name.

The fact that private members can influence a class interface type makes absolutely no sense to me.

jan-molak added a commit that referenced this issue May 13, 2017

feat(core): anonymous Tasks can be created using \`Task.where(descrip…
…tion, ...sub-tasks)\`

affects: serenity-js

Thanks to @InvictusMB for suggesting the TypeScript trick enabling the \`Task.where\` syntax in #22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment