fratzinger / object-replace-mustache

'mustache.js' but for replacing object properties
MIT License
4 stars 0 forks source link

Support for multiple (inline) properties #2

Closed miguelrk closed 9 months ago

miguelrk commented 2 years ago

Hey @fratzinger! Thanks for this very useful library.

Issue: I came across the need of using multiple (inline) properties e.g. { url: "https://example.com/{{version}}/{{resource}}" }, which I believe is a relatively common use case. However, this package currently only supports a single property (starting/ending with {{ and }} respectively e.g. "{{prop1}}".

My current workaround is simply wrapping the entire property in curly braces e.g. { url: "{{url}}" }.

I was wondering if this is a feature that would be viable for this package, since it might complicate the parsing logic (e.g. for array/object values). However, this can be handled simply by stringifying the value (despite it being array/object) and inserting it in the string. Some examples:

const original = { url: "https://example.com/v{{versionNumber}}/{{resource}}" }

// 1) primitive types (string, number, boolean, undefined, ...)
const viewPrimitives = { versionNumber: 1, resource: "users" }
replace(original, viewPrimitives) // returns { url: "https://example.com/v1/users" }

// 2) array/object types
const viewNonPrimitives = { versionNumber: { foo: 'bar' }, resource: ['users'] }
replace(original, viewNonPrimitives) // returns { url: "https://example.com/v{ foo: 'bar' }/['users']" }

NOTE: use case (2), handling array/object types in this way (inline) is certainly uncommon, however stringifying will ensure the rendering works also for these cases. This might be desired.

Is something like this worth adding to the library? If so, I think I could try to have a PR.

fratzinger commented 2 years ago

Hey @miguelrk, saw you starred the project and was existed what you're up to.

Case 1 is absolutely valid! I would love to see this in this package. I had thoughts against it which can be seen in the tests: https://github.com/fratzinger/object-replace-mustache/blob/main/test/object-replace-mustache.test.ts#L34 But I think I made these tests because I wanted a lightweight library for my limited use case.

Case 2 is more complex, I guess. The library is especially made for swapping {{ placeholders }} to it's real value. See test:

const transformed = replace("{{ test }}", { test: true });
assert.deepStrictEqual(transformed, true, "item changed");

It replaces the string with the value. This is where it's differs from mustache/handebars. I'm wondering how to keep this behavior. See: example 1:

const original = { url: "https://example.com/{{resource}}" }
const viewNonPrimitives = { resource: ['users'] }
replace(original, viewNonPrimitives) // returns { url: "https://example.com/['users']" }

makes perfectly sense. But what about example 2:

const original = { url: "{{resource}}" }
const viewNonPrimitives = { resource: ['users'] }
replace(original, viewNonPrimitives) // What do I expect?
// a stringified value?:
// { url: "['users']" }
// or the real value?:
// { url: ['users'] }
miguelrk commented 2 years ago

Thanks for the thourough answer!

Yeah, the fact that this library keeps the orignal data type is definitively one of its strengths compared to mustache and other (string) templating libraries which always cast to string.

I agree, in example 1 it makes sense to cast to string, since we are already telling that the inlined variable must be inserted to a string (type casting to a string is implicit).

Regarding example 2, I believe the proper default should remain not stringifying (the current behavior).

In that case, the only required update is to allow entries with values NOT starting with '{{' OR NOT starting with '}} (see relevant lines ), and instead of returning, stringify each item/value (could use .toString() method of array/object prototype) and insert it to the string in place.

These 2 cases can be summarized in the following table:

Case return Type example
item.startsWith("{{") && item.endsWith("}}") keep original type (current behavior) {{resource}} returns ['users'] (array)
!item.startsWith("{{") || !item.endsWith("}}") cast to string and inject a string {{resource}} returns "a string ['users']" (string) {{resource}} a string returns "['users'] a string" (string) a string {{resource}} another string returns a string "['users'] another string" (string)

So it wouldn't be that much of a change I think. Might I be missing something?

fratzinger commented 2 years ago

Sounds good to me! I'm pretty sure there come things along the way like nested stuff "{{ {{ test }} }}" test but I guess we can figure them out then.

My time is pretty limited right now and I don't have a use case for this at the moment but I would love to see this land in this package. I would be really happy if you'd work on an PR and of course I would assist. Please let me know, when you need me to do something.

Last question: Always fascinating as a package author: What is your use case for this package?

miguelrk commented 2 years ago

Of course! I'll get to it right now.

My use case is over at Netzo. We have a marketplace of re-usable plugins, each configured with a JSON object but different placeholder values. What we do is render a form in the UI every time a user wants to use an integration (the view object) and use that to "replace" the placeholder values with the corresponding ones.

So the use case is basically inserting user-specific config into templated config files in a very simple manner. We've noticed this approach is helps keep our plugins (their config files) very simple.

fratzinger commented 2 years ago

Nice. Thanks for the insight! πŸ™ Pretty interesting! So basically exactly what I thought it is useful for. I use it with feathers-casl to store permissions in the database. Or for feathers-trigger to store context based data in the database.

miguelrk commented 2 years ago

Hey @fratzinger , just reporting back. I struggled to get this to work since there ended up being more edge cases than I though 😞. This is something that I'd like to see though! So I hope I can get back to it soon, just got caught up at the moment. πŸ‘πŸΌ

fratzinger commented 2 years ago

Nevermind. πŸ‘ If you've got something worth keeping online, please feel free to make a draft PR so we don't forget about it.

miguelrk commented 2 years ago

Just very flacky tests and debugging code really, nothing worth submitting yet really πŸ˜Άβ€πŸŒ«οΈ .

fratzinger commented 9 months ago

Hey @miguelrk,

this is a long way coming and you probably won't need this anymore. Just for the record: this is possible now. I published a completely new v2 with a completely new parser. I added a test for your first example. See this:

https://github.com/fratzinger/object-replace-mustache/blob/9c7742b6be9384dafc1f16e4fef4fcbd74004386/test/object-replace.test.ts#L206-L218

Maybe it's useful for Netzo.

miguelrk commented 9 months ago

That's great to hear! Thanks for the heads up @fratzinger ! Will upgrade now and definitively, even the previous version has been very useful to us already!

fratzinger commented 9 months ago

Please beware that the new template engine is waaaaay more powerful and it's likely that there could be exploits. As long as you own the templates, you're totally save, but if you don't own the templates (user generated) or do not check the templates, be careful.

There are a few regexes to prevent exploits and restrict the access only to the 'view' object.

ejs (another template engine) says it best:

If you give end-users unfettered access to the EJS render method, you are using EJS in an inherently un-secure way. Please do not report security issues that stem from doing that.

EJS is effectively a JavaScript runtime. Its entire job is to execute JavaScript. If you run the EJS render method without checking the inputs yourself, you are responsible for the results.

miguelrk commented 9 months ago

Thanks for the heads up! I appreciate it, and I'll have that in mind πŸ‘πŸΌπŸ™ŒπŸ»