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

Implement <enter> and <leave> hooks as well as sub-tree skipping #54

Merged
merged 3 commits into from
Jan 4, 2021

Conversation

nikku
Copy link
Member

@nikku nikku commented Dec 29, 2020

A rule can now be explicit about when to check a given node, on enter, on leave or during both occasions:

module.exports = function someRule() {

  return {
    check: {
      enter: function(node, reporter) { ... },
      leave: function(node, reporter) { ... }
    }
  };
};

A rule may also return false from the enter hook to stop traversing deeper into the elements children.


Closes #52
Closes #53

Allows rules to specify enter and leave hooks:

```javascript
function someRule() {

  return {
    check: {
      enter: function(node, reporter) { ... },
      leave: function(node, reporter) { ... }
    }
  };
}
```

We still recognize the old behavior, exposing <enter> hook via a check
function.

Closes #52
Migrates remaining usages of <var> to <const>.
@adroste
Copy link

adroste commented Dec 30, 2020

tl;dr it's fine 👍


Use case 1

Search for matching types & perform checks after every element was visited.

Works by doing sth. like this:

function leave(node, reporter) {
    if (!is(node, 'bpmn:Definitions'))
        return;

   // calc & write reports
}

function enter(node, reporter) {
    // collect relevant elements
}

return { check: { enter, leave } };

Use case 2

Traversing/searching for inner element.

Works, example:

let startNode = null;
let innerElements = [];

function leave(node, reporter) {
    if (node !== startNode) 
        return;

    // do the checks

    startNode = null;
    innerElements = [];
}

function enter(node, reporter) {
    if (startNode) { 
        if (is(node, 'my:innerType')) {
            innerElements.push(node);
        }
    } 
    else if (is(node, 'my:outerType')) {
        startNode = node;
    }
}

return { check: { enter, leave } };

Use case 3

Early exit.

Works somewhat by short circuiting every enter. Example:

let finished = false;

function enter(node, reporter) {
    if (finished)
        return false;

    if (is(node, 'my:type')) {
        
        // do stuff...

        finished = true;
        return false;
    }
}

return { check: { enter } };

Conclusion

Works for me. Seems to be a good compromise between versatility and the need to write boilerplate code for certain use cases.

My tests did not encounter any unwanted behaviour.

As it does not break existing implementations, I think a merge should be safe.

Copy link
Contributor

@philippfromme philippfromme left a comment

Choose a reason for hiding this comment

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

Great contribution! 👏

@fake-join fake-join bot merged commit d26d6b4 into master Jan 4, 2021
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Jan 4, 2021
@fake-join fake-join bot deleted the hooks-subtree-skip branch January 4, 2021 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants