-
Notifications
You must be signed in to change notification settings - Fork 6
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: add textResolver #35
base: main
Are you sure you want to change the base?
feat: add textResolver #35
Conversation
Also do not forget DCO, thus all commits has to be signed-off. Read the details and follow instructions. |
Lets also update README with the information about the usage, and maybe some use cases. Optionally, we should extend DEMO app with the example of such resolver too. We can do some simple text replacement example or similar simple case just to illustrate that it is possible to hook into text node and affect it. |
0a6b348
to
fb8ee60
Compare
fb8ee60
to
490c922
Compare
content: text.replace("{name}", "World"), | ||
component: "span", | ||
props: { class: "class-2" }, | ||
}), |
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.
Hm, this does look weird honestly, and I am sure it clashes with the existing API. The problem is that conceptually the textResolver
should be meant for text resolving only, and should not return ComponentNode
at all. This function is not about the node logic at all, but rather about manipulating the content
property in whatever the node is.
to get same result you have described with solution we should better have the following API:
schema: {
nodes: {
text: () => ({
component: "p",
props: { class: "class-1" },
}),
},
},
textResolver: (text) => text.replace("{name}", "World")
(the textResolver extends the capability of the node, but does not override it)
Alternatively, we can expect the shape of textResolver?: (str: string) => Pick<ComponentNode, 'content'>;
and the usage:
textResolver: (text) => ({
content: text.replace("{name}", "World"),
}),
but again, this may misslead users that it is not the whole component node but rather just a content, so might be bad alternative.
What do you think?
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.
it should return ComponentNode
for cases when you example you want to support ICU syntax. So in this case all the texts need to be rendered inside some .astro
component
I can suggest to support both outputs:
textResolver?: (str: string) => ComponentNode | string;
Signed-off-by: Alexander Lozovsky <[email protected]>
490c922
to
056fb86
Compare
#67
Add
textResolver
to control the rendering of the plain text, to handle cases when you need to preprocess the text before rendering (translation, sanitization, etc)