Removed `view` property from page context

[jonschlinkert]

Removed `view` property from page context

[jonschlinkert]

hmm, strange. view should definitely be there. that's not something I would change without advance notice, then a minor bump and a big "heads up" in the changelog.

as far as I know, nothing has been removed from the context since templates was created. latest is 0.15.10, have you tried that version?

[dtothefp]

@jonschlinkert yes I tried templates 0.15.10 with assemble-core 0.14.3, still the same problem. can try to make you a small example

[jonschlinkert]

actually, I'm pretty sure view was never directly on the helper context (e.g. this.view) until recently. it was always on this.context.view. Now it's on both.

however... this.context.view is non-enumerable (I'm pretty sure it always was, but it's possible it was enumerable before, in which case this would be a regression).

I just now remembered something @doowb mentioning something about view, that might have been related. @doowb, maybe you remember?

[doowb]

I think I just realized what happened....

Before we returned the context object from bindHelpers and it was the same object that was used as the thisArg for helpers. Now, thisArg is different from the context that we're returning from bindHelpers.

I think we either need to add the view to the returned context (like in .compile) or add it to this context object

@dtothefp is having an issue because the context object passed into those nunjucks tags is the context that we pass into the render function... engine.render(view.contents, context). So it just needs to current view to be added.

I just now remembered something @doowb mentioning something about view, that might have been related. @doowb, maybe you remember?

This was a different issue related to the current view, but that was inside handlebars helpers and has been fixed.

[dtothefp]

Here's an example, run node ./index.js

Breaking
https://github.com/dtothefp/assemble-view-issue

Working
https://github.com/dtothefp/assemble-view-issue/tree/assemble-legacy

[jonschlinkert]

Before we returned the context object from bindHelpers

ahhh, damn. good insight @doowb. surprising that this isn't covered in like 1,200 unit tests

[doowb]

however... this.context.view is non-enumerable (I'm pretty sure it always was, but it's possible it was enumerable before, in which case this would be a regression).

This is what's happening.

The .view property is on the context just like before (I missed where the reference was still kept throughout the bindHelpers method.

The issue is that it looks like nunjucks doesn't keep non-enumerable properties on the context. When I do this.context.view = view in the Context object, I see the view in the nunjucks tag, but I'm not able to even get to the view when doing ctx.view in the tag, so it must be removing it.

[doowb]

I even tried adding the view to the context passed in when rendering and it didn't show up. Nunjucks must bind the context from the compile function and only use that (unless there's something else I don't know about nunjucks, which is probably the case)

[jonschlinkert]

if you change it to this.context.view instead of using define did it work?

[doowb]

if you change it to this.context.view instead of using define did it work?

Yes

[jonschlinkert]

oh yeah sorry I see where you said that already. thx

[dtothefp]

@doowb @jonschlinkert hey guys so what does this all mean? I don't want you to have to change your code just to accommodate nunjucks. Is this something I just shouldn't count on in the future and just bind the view context I need in an onload middleware or something?

[doowb]

hey guys so what does this all mean?

Nunjucks only "sees" enumerable properties when they make a copy of the context passed in. Since we're doing something like...

Object.defineProperty(context, 'view', {
  configurable: true,
  enumerable: false,
  value: view
});

...nunjucks doesn't "see" the view property and doesn't copy it over to the context.ctx property in your tag.

just bind the view context I need in an onload middleware or something?

The following using preCompile works for nunjucks.

app.create('pages', {renameKey})
  .use(loader())
  .preCompile(/./, function(file, next) {
    file.data.view = file;
    next();
  });

This lets you use whatever property you want and put other things on the file.data object that will be accessible in your tags.

As @jonschlinkert said above...

however... this.context.view is non-enumerable (I'm pretty sure it always was, but it's possible it was enumerable before, in which case this would be a regression).

...so maybe we should make it non-enumerable.

FWIW... I found this issue on nunjucks that talks about es2105 getter syntax not working (which I think transpiles into Object.defineProperty). Looks like they'd be willing to take a PR if you find a fix 😉

[jonschlinkert]

...so maybe we should make it non-enumerable.

@doowb meant "make it enumerable". yeah let's do that. I haven't looked at the nunjucks internals, but I imagine they'll need to do more than a simple object-assign on the context. getters should also be copied

[jonschlinkert]

I don't want you to have to change your code just to accommodate nunjucks.

meant to reply to this. if view was enumerable before, the fact that behavior has changed for you makes this a regression. we'll make view enumerable, but in the meantime hopefully you are able to solve it for now using the solution @doowb posted

[jonschlinkert]

closing since I think this was resolved a while ago. please feel free to reopen if there is still an issue