fix: adding vitest and initial tagging function [TOL-909] #5
Conversation
| locale, | ||
| }); | ||
| expect(result).toStrictEqual({ | ||
| 'data-contentful-field-id': `${entryId}:${fieldId}:${locale}`, |
There was a problem hiding this comment.
Just thinking out loud here, what if the user has a ':' inside of the fieldId? Won't this cause issues to extract the variables from the string?
We had a similar issue with the path in Compose before
There was a problem hiding this comment.
Yeah defo a good point! I didn't want to reintroduce the 👺 or what we had before. Can you think of a more sensible delimiter?
There was a problem hiding this comment.
I guess another way to do it could be
'data-contentful-field-id': fieldId,
'data-contentful-entry-id': entryId,
'data-contentful-locale': locale`,
then no need for the delimiting
There was a problem hiding this comment.
Maybe a delimiter that is less likely to appear within the variable values? Like the pipe symbol (|) or tilde symbol (~)? Not sure...
There was a problem hiding this comment.
Ah yes I like your approach 👏
There was a problem hiding this comment.
Just one note:
That could be resolve to a lot of markup on the website and bloat the HTML response (on server-side-rendering).
I guess we should give some best practices to customers to only render the tags on a preview system so they don't affect the performance of their website?
There was a problem hiding this comment.
@chrishelgert yeah makes sense!
|
🎉 This PR is included in version 1.0.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Adding initial tagging functionality