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

Remove action bubbling #61

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 5 additions & 23 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,38 +7,20 @@ Demo: http://jsbin.com/jipani/edit?html,js,output
ember install ember-route-action-helper
```

The `route-action` helper allows you to bubble closure actions, which will delegate it to the currently active route hierarchy per the bubbling rules explained under `actions`. Like closure actions, `route-action` will also have a return value.
The `route-action` helper allows you to call actions from routes. Like closure actions, `route-action` will also have a return value.

However, returning `true` in an action will **not** preserve bubbling semantics. In case you would like that behavior, you should use ordinary string actions instead.
Route actions do not bubble. In case you would like that behavior, you should use ordinary string actions instead.

## Usage

For example, this route template tells the component to lookup the `updateFoo` action on the route when its internal `clicked` property is invoked, and curries the function call with 2 arguments.
For example, this route template tells the component to lookup the `updateFoo` action on the current route when its internal `clicked` property is invoked, and curries the function call with 2 arguments.

```hbs
{{! foo/route.hbs }}
{{foo-bar clicked=(route-action "updateFoo" "Hello" "world")}}
```

If the action is not found on the current route, it is bubbled up:

```js
// application/route.js
import Ember from 'ember';

const { Route, set } = Ember;

export default Route.extend({
actions: {
updateFoo(...args) {
// handle action
return 42;
}
}
});
```

If no action is found after bubbling, an error will be raised. The `route-action` also has a return value:
If no action is found, an error will be raised. The `route-action` also has a return value:
Copy link
Contributor

Choose a reason for hiding this comment

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

Might want to say "found in the current route"?


```js
// foo/component.js
Expand All @@ -65,7 +47,7 @@ You may also use in conjunction with the `{{action}}` helper:

## Compatibility

This addon will work on Ember versions `1.13.x` and up only, due to use of the new `Helper` implementation.
This addon will work on Ember versions `2.x` and up only.
Copy link
Contributor

Choose a reason for hiding this comment

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

If we are going for "breaking changes" maybe we can just say 2.4?


## Installation

Expand Down
1 change: 1 addition & 0 deletions addon/-private/internals.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// @ts-nocheck
Copy link
Contributor

Choose a reason for hiding this comment

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

What's this do?

import Ember from 'ember';

let ClosureActionModule;
Expand Down
93 changes: 77 additions & 16 deletions addon/helpers/route-action.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,38 +2,94 @@ import Ember from 'ember';
import { ACTION } from '../-private/internals';

const {
A: emberArray,
Helper,
assert,
canInvoke,
computed,
typeOf,
get,
getOwner,
run,
isPresent,
// @ts-ignore no type signature
runInDebug
} = Ember;

/**
* @typedef {Object} RouterInstance
* @property {Router} _routerMicrolib
* @property {Router} router
* @property {string} currentRouteName
*/
/**
* @typedef {Object} Router
* @property {Handler[]} currentHandlerInfos
* */
/**
* @typedef {Object} Handler
* @property {string} name
* @property {Route} handler
* */
/**
* @typedef {Object} Route
* @property {ActionsHash} actions
* @property {ActionsHash} _actions
* */
/**
* @typedef {Object} ActionsHash
* @property {[propName: string]: function(): void}
*/
/**
* @typedef {Object} RouteAction
* @property {function(): void} action
* @property {Route} handler
* */

/**
* Get current handler infos from the router.
*
* @param {RouterInstance} router
* @returns {Handler[]}
*/
function getCurrentHandlerInfos(router) {
let routerLib = router._routerMicrolib || router.router;

return routerLib.currentHandlerInfos;
}

function getRoutes(router) {
return emberArray(getCurrentHandlerInfos(router))
.mapBy('handler')
.reverse();
/**
* Get current handler instances from router.
*
* @param {RouterInstance} router
* @returns {Route[]}
*/
function getCurrentHandlers(router) {
/** @type {string} */
let currentRouteName = get(router, 'currentRouteName');
let currentRoot = currentRouteName.replace(/\.index$/gi, '');
return getCurrentHandlerInfos(router)
.reduce((acc, h) => {
return h.name === currentRouteName || h.name === currentRoot
? [h.handler, ...acc]
: acc;
}, []);
}

function getRouteWithAction(router, actionName) {
/**
* Get current route handler and action.
*
* @param {RouterInstance} router
* @param {string} actionName
* @returns {RouteAction}
*/
function getCurrentRouteWithAction(router, actionName) {
/** @type {function(): void} */
let action;
let handler = emberArray(getRoutes(router)).find((route) => {
/** @type {Route} */
let handler = getCurrentHandlers(router).find((route) => {
/** @type {ActionsHash} */
let actions = route.actions || route._actions;
action = actions[actionName];

return typeOf(action) === 'function';
return canInvoke(actions, actionName);
});

return { action, handler };
}

Expand All @@ -42,17 +98,22 @@ export default Helper.extend({
return getOwner(this).lookup('router:main');
}).readOnly(),

/**
* @param {any} [actionName, ...params]
* @returns {function(...invocationArgs: any[]): void}
*/
compute([actionName, ...params]) {
/** @type {RouterInstance} */
let router = get(this, 'router');
assert('[ember-route-action-helper] Unable to lookup router', router);
assert('[ember-route-action-helper] Unable to lookup router', isPresent(router));

runInDebug(() => {
let { handler } = getRouteWithAction(router, actionName);
assert(`[ember-route-action-helper] Unable to find action ${actionName}`, handler);
let { handler } = getCurrentRouteWithAction(router, actionName);
assert(`[ember-route-action-helper] Unable to find action ${actionName}`, isPresent(handler));
});

let routeAction = function(...invocationArgs) {
let { action, handler } = getRouteWithAction(router, actionName);
let { action, handler } = getCurrentRouteWithAction(router, actionName);
let args = params.concat(invocationArgs);
return run.join(handler, action, ...args);
};
Expand Down
16 changes: 0 additions & 16 deletions config/ember-try.js
Original file line number Diff line number Diff line change
@@ -1,22 +1,6 @@
/* eslint-env node */
module.exports = {
scenarios: [
{
name: 'ember-1.13',
bower: {
dependencies: {
'ember': '~1.13.0'
},
resolutions: {
'ember': '~1.13.0'
}
},
npm: {
devDependencies: {
'ember-source': null
}
}
},
{
name: 'ember-lts-2.4',
bower: {
Expand Down
6 changes: 4 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"scripts": {
"build": "ember build",
"start": "ember server",
"test": "ember try:each"
"test": "tsc && ember try:each"
},
"repository": "https://github.com/DockYard/ember-route-action-helper",
"engines": {
Expand All @@ -21,6 +21,7 @@
],
"license": "MIT",
"devDependencies": {
"@types/ember": "^2.7.41",
"broccoli-asset-rev": "^2.4.5",
"ember-ajax": "^3.0.0",
"ember-cli": "2.13.0",
Expand All @@ -40,7 +41,8 @@
"ember-load-initializers": "^1.0.0",
"ember-resolver": "^4.0.0",
"ember-source": "~2.13.0",
"loader.js": "^4.2.3"
"loader.js": "^4.2.3",
"typescript": "^2.3.2"
},
"keywords": [
"ember-addon",
Expand Down
58 changes: 32 additions & 26 deletions tests/acceptance/main-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,29 +26,44 @@ moduleForAcceptance('Acceptance | main', {
}
});

test('it bubbles a route action', function(assert) {
visit('/thing');
test('it has a return value', function(assert) {
visit('/math');

andThen(() => assert.equal(currentURL(), '/thing'));
andThen(() => click('.foo-button'));
andThen(() => assert.equal(findWithAssert('.foo-value').text().trim(), 'Hello world Bob!'));
andThen(() => assert.equal(currentURL(), '/math'));
andThen(() => click('#math-1'));
andThen(() => assert.equal(findWithAssert('#math-value').text().trim(), '3'));
andThen(() => click('#math-2'));
andThen(() => assert.equal(findWithAssert('#math-value').text().trim(), '16'));
andThen(() => click('#math-3'));
andThen(() => assert.equal(findWithAssert('#math-value').text().trim(), '15'));
andThen(() => click('.confirm-value-button'));
andThen(() => assert.equal(findWithAssert('.confirm-value').text().trim(), 'My value is 25'));
});

test('it has a return value', function(assert) {
visit('/thing');
test('it can be partially applied', function(assert) {
visit('/math');

andThen(() => assert.equal(currentURL(), '/thing'));
andThen(() => click('.thing .max-button'));
andThen(() => assert.equal(findWithAssert('.thing .max-value').text().trim(), '20'));
andThen(() => click('.add-value-button'));
andThen(() => assert.equal(findWithAssert('.add-value').text().trim(), 'My value is 7'));
});

// changing routes, 2 helpers invoked
andThen(() => visit('/thing/show'));
andThen(() => assert.equal(currentURL(), '/thing/show'));
andThen(() => click('.thing-show .max-button'));
test('it invokes action in the current route hierarchy', function(assert) {
visit('/math');
andThen(() => click('#math-1'));
andThen(() => assert.equal(findWithAssert('#math-value').text().trim(), '3'));
visit('/math/add');
andThen(() => click('#math-add-1'));
andThen(() => assert.equal(findWithAssert('#math-add-value').text().trim(), '[math/add] Value is: 3'));
});

// ensure values are different
andThen(() => assert.equal(findWithAssert('.thing .max-value').text().trim(), '20'));
andThen(() => assert.equal(findWithAssert('.thing-show .max-value').text().trim(), '300'));
test('it handles .index routes', function(assert) {
visit('/hello');
andThen(() => click('#hello-index-button'));
andThen(() => assert.equal(findWithAssert('#hello-index-value').text().trim(), 'Hello from hello.index'));
andThen(() => click('#hello-button-1'));
andThen(() => assert.equal(findWithAssert('#hello-value').text().trim(), '', 'should not fire because `hello.index` action takes precedence'));
andThen(() => click('#hello-button-2'));
andThen(() => assert.equal(findWithAssert('#hello-value').text().trim(), 'HELLO FROM HELLO'));
});

test('it can be used without rewrapping with (action (route-action "foo"))', function() {
Expand All @@ -68,12 +83,3 @@ skip('it should throw an error immediately if the route action is missing', func
// });
// });
});

test('it invokes action in the current route hierarchy', function(assert) {
visit('/thing');
click('.foo-button');
andThen(() => assert.equal(findWithAssert('.foo-value').text().trim(), 'Hello world Bob!'));
visit('/thing/route-with-action');
click('.foo-button');
andThen(() => assert.equal(findWithAssert('.foo-value').text().trim(), 'Set via route-with-action: Hello world Bob!'));
});
13 changes: 13 additions & 0 deletions tests/dummy/app/components/add-value.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import Ember from 'ember';

const { Component } = Ember;

export default Component.extend({
baseValue: 4,
value: null,
actions: {
addValue(x) {
this.set('value', this.get('add')(x));
}
}
});
12 changes: 12 additions & 0 deletions tests/dummy/app/components/confirm-value.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import Ember from 'ember';

const { Component } = Ember;

export default Component.extend({
value: null,
actions: {
confirm() {
this.set('value', this.get('doAction')());
}
}
});
13 changes: 0 additions & 13 deletions tests/dummy/app/components/foo-bar.js

This file was deleted.

8 changes: 5 additions & 3 deletions tests/dummy/app/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ const Router = Ember.Router.extend({
});

Router.map(function() {
this.route('thing', function() {
this.route('show');
this.route('route-with-action');
this.route('math', function() {
this.route('add');
});
this.route('hello', function() {
this.route('world');
})
this.route('dynamic');
this.route('dynamic2');
});
Expand Down
15 changes: 0 additions & 15 deletions tests/dummy/app/routes/application.js

This file was deleted.

Loading