Conversation
cpinn
force-pushed
the
caitlin/modify-nunjucks-parsing-to-match-custom-mustache-behavior
branch
from
97a2bde to
74e0bc1
Compare
js/src/template/nunjucks-env.ts
Outdated
| throwOnUndefined, | ||
| }); | ||
|
|
||
| // Intercept renderString to wrap objects with custom toString |
There was a problem hiding this comment.
nunjucks doesn't really support the type of custom escaping we were doing for mustache. This implements some of the custom parsing we were doing just to make sure that [Object object] doesn't get printed out during the toString method. Kind of hacky but it works to match the behavior.
cpinn
force-pushed
the
caitlin/modify-nunjucks-parsing-to-match-custom-mustache-behavior
branch
from
77b2ee2 to
d6a79e6
Compare
| return new nunjucks.Environment(null, { | ||
| autoescape: true, | ||
| const env = new nunjucks.Environment(null, { | ||
| autoescape: false, |
There was a problem hiding this comment.
i'd double check our docs to make sure we call out that we don't escape by default in both template renderers. perhaps how to escape, for those that want to
There was a problem hiding this comment.
We don't call this out at all, never did which is why I was quite surprised by it.
js/src/template/renderer.test.ts
Outdated
| { templateFormat: "nunjucks" }, | ||
| ); | ||
| // Arrays output as comma-separated (Nunjucks default behavior) | ||
| expect(resultWithAccess).toBe("User: Bob, Age: 25, Items: 1,2,3"); |
There was a problem hiding this comment.
i expected the array to be ["1", "2", "3"] 🤔 (json stringify)
There was a problem hiding this comment.
I haven't decided if I want to overwrite this or not because this is just the behavior of nunjucks.
if you have an array it doesn't get output as [object object] it just gets outputs the array as such, this is regular rendering. Its a different behavior than mustache above which would print out the array.
There was a problem hiding this comment.
looks like mustache does do as I'd expected. 🤔
cpinn
changed the title
cpinn
force-pushed
the
caitlin/modify-nunjucks-parsing-to-match-custom-mustache-behavior
branch
from
22803a9 to
5df5f53
Compare
cpinn
force-pushed
the
caitlin/modify-nunjucks-parsing-to-match-custom-mustache-behavior
branch
from
5bc4989 to
2c9a086
Compare
…tom-mustache-behavior
…tom-mustache-behavior
| if (!Array.isArray(value)) return null; | ||
|
|
||
| // Check if array contains attachments or image URLs (supports mixed arrays) | ||
| const allValid = value.every( |
There was a problem hiding this comment.
The flow is that the backend will attempt to turn all the references into URLs. Now if something goes wrong and one of the attachments isn't turned into a URL this would still allow expansion by ensuring the other values are attachments.
A different flow I experimented with was passing the attachment map from the backend for verification. I did not like that mixing of concerns though so I did not think it was a good idea to introduce that logic here.
2 participants
Users would like to be able to reference an array when handling images.
This PR introduces the following changes:
Variables that contain arrays of images or files will now be automatically expanded when used.
Nunjucks is updated to mirror what was done for Mustache html parsing, escaping is now disabled in the Nunjucks environment.
Mustache JSON-stringifies objects by default. For Nunjucks, we preserve its native behavior and do not JSON-stringify by default. Instead, a tojson filter has been added so that users familiar with Nunjucks or Jinja can explicitly convert data to JSON using standard renderer formats.
Note: I went back and forth on whether to keep Mustache’s default behavior of automatically calling JSON.stringify on objects. Ultimately, I chose not to replicate this in Nunjucks. Since Nunjucks is a different templating language than Mustache, this behavioral difference made more sense to me.