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.

Sign up for GitHub

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

Jump to bottom

{?exists} and {^exists} now supports values from a promise #753

Merged
merged 7 commits into from
Nov 9, 2016
Merged

{?exists} and {^exists} now supports values from a promise #753

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7a120e6
{?exists} and {^exists} now supports values from a promise
Nov 7, 2016
71fdcdc
Updated templates test so that it correctly reports errors as "expect…
Nov 8, 2016
dcc36d0
Updated {?exists} and {^exists} logic to support promises and report …
Nov 8, 2016
932d5f6
Created mapThenable function that is just like Chunk#map but should b…
Nov 9, 2016
6cc5a3c
Updated Chunk#stream to use Chunk#renderError.
Nov 9, 2016
337ef1a
Removed unnecessary semicolon
Nov 9, 2016
c7a4952
Updated spacing so that functions have a space before opening curly b…
Nov 9, 2016
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
51 changes: 41 additions & 10 deletions lib/dust.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -861,6 +861,17 @@
var body = bodies.block,
skip = bodies['else'];

if (dust.isThenable(elem)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you turn this into a little reusable func that we can use for exists and notexists both? Look above at getWithResolvedData as an example-- just a little helper func not attached to the Chunk prototype.

Copy link
Contributor

Choose a reason for hiding this comment

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

by "this" I meant the code inside the if() block that is

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code was definitely redundant between the two methods. I made a mapThenable method to address it.

return this.map(function(chunk) {
elem.then(function(data) {
chunk.exists(data, context, bodies).end();
}, function(err) {
dust.log('Unhandled promise rejection in `' + context.getTemplateName() + '`', INFO);
chunk.renderError(err, context, bodies).end();
});
});
}

if (!dust.isEmpty(elem)) {
if (body) {
return body(this, context);
Expand All @@ -876,6 +887,17 @@
var body = bodies.block,
skip = bodies['else'];

if (dust.isThenable(elem)) {
return this.map(function(chunk) {
elem.then(function(data) {
chunk.notexists(data, context, bodies).end();
}, function(err) {
dust.log('Unhandled promise rejection in `' + context.getTemplateName() + '`', INFO);
chunk.renderError(err, context, bodies).end();
});
});
}

if (dust.isEmpty(elem)) {
if (body) {
return body(this, context);
Expand Down Expand Up @@ -973,24 +995,33 @@
return this.map(function(chunk) {
thenable.then(function(data) {
if (bodies) {
chunk = chunk.section(data, context, bodies);
chunk.section(data, context, bodies).end();
} else {
// Actually a reference. Self-closing sections don't render
chunk = chunk.reference(data, context, auto, filters);
chunk.reference(data, context, auto, filters).end();
}
chunk.end();
}, function(err) {
var errorBody = bodies && bodies.error;
if(errorBody) {
chunk.render(errorBody, context.push(err)).end();
} else {
dust.log('Unhandled promise rejection in `' + context.getTemplateName() + '`', INFO);
chunk.end();
}
dust.log('Unhandled promise rejection in `' + context.getTemplateName() + '`', INFO);
chunk.renderError(err, context, bodies).end();
});
});
};

/**
* Render an error body if available
* @param thenable {Thenable} the target thenable to await
Copy link
Contributor

Choose a reason for hiding this comment

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

wrong param

* @param context {Context} context to use to render the error
* @param bodies {Object} may optionally contain an "error" which will be rendered
* @return {Chunk}
*/
Chunk.prototype.renderError = function(err, context, bodies) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, good reusable func

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Should I move it off of the Chunk object? When I added it originally I wasn't considering that I was updating a public api that's pretty well documented here: http://www.dustjs.com/docs/helper-api/

var errorBody = bodies && bodies.error;
if (errorBody) {
return this.render(errorBody, context.push(err));
}
return this;
};

/**
* Reserve a chunk to be evaluated with the contents of a streamable.
* Currently an error event will bomb out the stream. Once an error
Expand Down
6 changes: 3 additions & 3 deletions test/templates.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ function render(test, dust) {
expect(messageInLog(dust.logQueue, test.log)).toEqual(true);
}
if (typeof test.expected !== 'undefined') {
expect(test.expected).toEqual(output);
expect(output).toEqual(test.expected);
}
done();
};
Expand Down Expand Up @@ -124,7 +124,7 @@ function stream(test, dust) {
expect(messageInLog(dust.logQueue, test.log)).toEqual(true);
}
if (typeof test.expected !== 'undefined') {
expect(test.expected).toEqual(result.output);
expect(result.output).toEqual(test.expected);
}
done();
};
Expand Down Expand Up @@ -196,7 +196,7 @@ function pipe(test, dust) {
expect(messageInLog(dust.logQueue, test.log)).toEqual(true);
}
if (typeof test.expected !== 'undefined') {
expect(test.expected).toEqual(result.data);
expect(result.data).toEqual(test.expected);
}
if(calls === 2) {
done();
Expand Down
19 changes: 19 additions & 0 deletions test/templates/all.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,18 @@ return [
message: "should setup base template for next test. hi should not be part of base block name"

},
{
name: "{?exists} supports promises and uses correct context",
source: "{#a}{?b}{test}{/b}{/a}",
context: {
a: {
b: FalsePromise(null, { test: "BAD" }),
test: "GOOD"
}
},
expected: "GOOD",
message: "{?exists} supports promises and uses correct context",
},
{
name: "issue322 use base template picks up prefix chunk data",
source: '{>issue322 name="abc"/}' +
Expand Down Expand Up @@ -584,6 +596,13 @@ return [
expected: "false",
message: "empty array is treated as empty in exists"
},
{
name: "empty array resolved from a Promise is treated as empty in exists",
source: "{?emptyArrayFromPromise}true{:else}false{/emptyArrayFromPromise}",
context: {"emptyArrayFromPromise": FalsePromise(null, [])},
expected: "false",
message: "empty array resolved from a Promise is treated as empty in exists"
},
{
name: "empty {} is treated as non empty in exists",
source: "{?object}true{:else}false{/object}",
Expand Down