Skip to content
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

Usage of renderToStaticMarkup from react-dom/server in react Text component bloats client filesize #107

Open
morbidick opened this issue Sep 30, 2022 · 7 comments

Comments

@morbidick
Copy link

Hi, thank you for this nice and small translation library.

In commit 3c1d0a0 renderToStaticMarkup from react-dom/server was introduced to the Text Component, unfortunately that leads to the inclusion for react-dom-server in the client build and bloating the filesize.

@pret-a-porter
Copy link
Collaborator

@morbidick Yes it is, but it allows to use React components as parameters for Text component

@appbak3r
Copy link

@pret-a-porter That's actually an issue for vite for example, having react-dom/server increases bundle size by 25% and it makes no sense to use the library

@lubieowoce
Copy link

lubieowoce commented Apr 28, 2023

Yes it is, but it allows to use React components as parameters for Text component

@pret-a-porter Right, but that component won't have access to things like context, because it's rendered in isolation. That feels like a pretty big footgun.

@pret-a-porter
Copy link
Collaborator

Make sense, issue reopened

@pret-a-porter pret-a-porter reopened this May 25, 2023
@lubieowoce
Copy link

lubieowoce commented May 26, 2023

AFAIK the "proper" way to handle it is to make Text return a fragment, and let react handle the rest. So, something like the following:

// messages: { hello: 'Hello, {name}. Nice to meet you!' }

// a Text like this:
<Text id="hello" name={<SomeComponent />} />

// should output (return) this:
<>{"Hello, "}<SomeComponent />{". Nice to meet you!"}</>

@pret-a-porter
Copy link
Collaborator

Unfortunately it does not cover case, when string contains html tags. React fragments does not support dangerouslySetInnerHTML and this proposal reactjs/rfcs#129 still in RFC stage for more than two years.

@lubieowoce
Copy link

yes, html strings would probably need to be parsed into a react tree (a la react-html-parser). not ideal, but i don't think there's a way around it if you want to have <Elements> work properly. i guess at least its cacheable (the translations don't really change much) so shouldn't be a big perf problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants