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

feat(core): Improved control flow with times and stopPropagation #202

Merged
merged 8 commits into from
Apr 27, 2019

Conversation

offirgolan
Copy link
Collaborator

@offirgolan offirgolan commented Apr 18, 2019

Description

This PR includes 2 features that when used together, can be really powerful.

The first is the ability to remove an intercept or event handler after a specified amount of times it has been called via the handler's .times(n) API or by providing it as an option to .on(eventName, fn, { times }) or .intercept(fn, { times }).

The second, is the ability to prevent calling the next handlers registered for the events or intercept. This can be done via interceptor.stopPropagation() or via a new Event instance passed to every event handler by calling event.stopPropagation().

Motivation and Context

Lets take an example where we might want to intercept a user being removed. The first call to GET the user should return the user's info but after we delete the user, it should return a 404.

// First call should return the user and not enter the 2nd handler
server
  .get('/users/1')
  .times(1) // Remove this interceptor after it gets called once
  .intercept((req, res, interceptor) => {
    // Do not continue to the next intercept handler which handles the 404 case
    interceptor.stopPropagation(); 
    res.sendStatus(200);
  });

server.delete('/users/1').intercept((req, res) => res.sendStatus(204));

// Second call should 404 since the user no longer exists
server
  .get('/users/1')
  .intercept((req, res) => res.sendStatus(404));

Then when the following requests get made, we are able to simulate the correct responses:

await fetch('/users/1'); // --> 200
await fetch('/users/1', { method: 'DELETE' }); // --> 204
await fetch('/users/1');  // --> 404

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have added tests to cover my changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • My code follows the code style of this project.
  • My commits and the title of this PR follow the Conventional Commits Specification.
  • I have read the contributing guidelines.

@offirgolan offirgolan added enhancement ✨ New feature or request 📦 core labels Apr 18, 2019
@offirgolan offirgolan requested a review from jasonmit April 18, 2019 16:58
@@ -201,6 +201,42 @@ describe('Integration | Server', function() {
/Invalid filter callback provided/
);
});

it('.times()', async function() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth adding a test for the precedence between times() + { times } when both are used

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@@ -11,7 +13,9 @@ export default class Handler extends Map {
super();

this.set('config', {});
this.set('defaultOptions', {});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why the name defaultOptions?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its more future proofing than anything else. Will allow us to add more API to modify default options passed to the interceptor/events.

@offirgolan offirgolan merged commit 2c8231e into master Apr 27, 2019
@offirgolan offirgolan deleted the cancel-method-after-n-times branch April 27, 2019 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ New feature or request 📦 core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants