-
Notifications
You must be signed in to change notification settings - Fork 7
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
feat(core): expressive content replacement #139
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: aee0091 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like using existing standards for this.
however I'm a bit hesitant of this exact implementation, as
- mustache hasn't had a commit in a year
- I'm concerned about how many functions will be needed to support all the edge cases
however I think it'll be reasonably easy to change later so I'm ok with going with it
import Mustache from "mustache"; | ||
|
||
// Prevent default HTML escaping behaviour | ||
Mustache.escape = function (value) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overriding globals is probably not a good idea given that this code is executed in the same context as other apps
return target.replace(/{{([^{{}}]+)}}/g, (_, key) => get(context, key, ``)); | ||
const output = Mustache.render(target, { | ||
...context, | ||
decimals: function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with this pattern, this list of functions is going to grow very long and it'll be bundled into every app.
I agree with your evaluation. Another gripe I have with mustache is that the syntax is quite verbose. The ideal library for our use case is lightweight and has a comprehensive library of helper built-in helper functions.
A possible course of action could be to go ahead with mustache, take inspiration from existing other long-standing analogs like the python's I think this is a necessary feature if we want to make rich client-only logic possible - but not strictly necessary if we work under the assumption that every mod will have a backend which can handle this kind of transformation logic. |
Change Summary
This PR proposes a standard syntax for string manipulation inside Mod elements by adopting mustache.js. This library is lightweight at 6.5kb minified or 2.7kb minified+gzipped (bundlephobia) and uses a familiar
{{}}
syntax for replacement as well aspath.to.value
syntax for accessing nested context values, making it backwards compatible.Arbitrary functions can be exposed to the mustache syntax in the
replaceInlineContext
function by passing anonymous function builders along with the context to the rendering function (see theLambdas
section in the manual and spec).An example adjustment to the
replaceInlineContext
function which introduces adecimals
operator which displays a number to a fixed number of decimals would look like this:And would be used like this:
Merge Checklist