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

Updated with fix for EJS templates rendering as document fragments #37

Closed
wants to merge 1 commit into from
Closed

Updated with fix for EJS templates rendering as document fragments #37

wants to merge 1 commit into from

Conversation

WooFerPPK
Copy link

@WooFerPPK WooFerPPK commented Apr 9, 2019

Worked with @justinbmeyer to come up with a solution to the [Object DocumentFragment] string being displayed on some instances where EJS was being used.

@@ -171,5 +751,22 @@ deepAssign($view, {
}
});

$view.register({
Copy link
Contributor

Choose a reason for hiding this comment

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

This register should be done in can-ejs


// If we are `async`...
if (async) {
// Return the deferred
Copy link
Contributor

Choose a reason for hiding this comment

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

this should warn that this is not supported

var getRenderer = function (obj, async) {
// If `obj` already is a renderer function just resolve a Deferred with it
if(isFunction(obj)) {
var def = can.Deferred();
Copy link
Contributor

Choose a reason for hiding this comment

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

} else {
// Make an ajax request for text.
var d = new can.Deferred();
jQuery.ajax({
Copy link
Contributor

Choose a reason for hiding this comment

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

can.ajax support async: false?

Copy link
Contributor

Choose a reason for hiding this comment

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

Will need to update https://github.com/canjs/can-ajax both 1.X and 2.X versions with this.

renderer = makeRenderer( info.renderer(id, text) );
}

def = def || new can.Deferred();
Copy link
Contributor

Choose a reason for hiding this comment

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

{
  then: function(){ return renderer },
  state: function(){ return "resolved" }
}

*/
renderTo: function(format, renderer, data, helpers, nodelist){
if (format === "string") {
if (renderer.renderAsString) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This will check for the actual string renderer now, which is provided by ejs here:

https://github.com/canjs/can-ejs/pull/67/files#diff-d693f1cbebb75316de0f7e3e21837a56R23

@justinbmeyer
Copy link
Contributor

  1. Add async to can-ajax
  2. Copy the deferred "polyfill" into this repo and import and use it.
  3. Move the .register call into can-ejs
  4. Look for all other uses of can.X and if possible, import the method directly.
  5. Add a test for calling can.view.renderer() .. does it render as a string?
  6. Do other tests work? Hopefully.
  7. Is there any code paths that are not supported (like deferrreds stuff), remove and replace with an error warning.
  8. Publish and celebrate.

@bmomberger-bitovi
Copy link
Contributor

Closing in favor of #38 (same but with extra fixes).

bmomberger-bitovi added a commit that referenced this pull request Apr 11, 2019
Update of PR #37 to support EJS templates rendering as strings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants