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

[react-intl] Use of "defaultRichTextElements" print error directly to console without using "onError" #2258

Closed
javebers opened this issue Oct 29, 2020 · 19 comments

Comments

@javebers
Copy link

javebers commented Oct 29, 2020

Which package?
react-intl 5.8.8

Describe the bug
When using "defaultRichTextElements" an error is printed directly to console for each formatted message without using "onError", so it can't be caught. I think that console error should be avoidable since using pre-compiled messages is a suggestion and not mandatory.

Error message: "[@formatjs/intl] "defaultRichTextElements" was specified but "message" was not pre-compiled.
Please consider using "@formatjs/cli" to pre-compile your messages for performance.
For more details see https://formatjs.io/docs/getting-started/message-distribution"

To Reproduce
Steps to reproduce the behavior:

  • Add any defaultRichTextElements configuration to IntlProvider without pre-compiled messages
    <IntlProvider
        locale={locale}
        messages={messages}
        onError={(error) => { /* do nothing */ } }
        defaultRichTextElements={{
            b: (chunks) => <b>{chunks}</b>,
            br: () => <br />
        }}
    >
  • Use <FormattedMessage {...} />

Expected behavior
No react-intl error should be printed on console

Desktop (please complete the following information):

  • OS: Windows 10
  • Browser: Chrome
  • Version: 84
@javebers javebers added the bug label Oct 29, 2020
@longlho
Copy link
Member

longlho commented Oct 29, 2020

Any specific reason preventing you from precompiling?

@javebers
Copy link
Author

javebers commented Oct 29, 2020

Not really. I honestly didn't know until I saw the error that there was a possibility to use a precompiler, so I appreciate the suggestion. But until I make time to check it out, it can be a bit annoying to have an error message every time FormattedMessage is used.

@longlho
Copy link
Member

longlho commented Oct 30, 2020

yeah I understand but that's kinda the reason why we left it there as a console.error 😄

@javebers
Copy link
Author

Sure, it serves its purpose that it's like this by default, but I still think one should be able to silence the error message. Or at least the error appears once and not every time a formatted message is used, to make it less invasive.

@longlho
Copy link
Member

longlho commented Oct 30, 2020

I'll keep this one open and if there's enough people wanting it we'll make the change :)

@longlho longlho added enhancement and removed bug labels Oct 30, 2020
@electerious
Copy link
Contributor

Is it correct that defaultRichTextElements prints an error, but adding the same mapping to FormattedMessage via values doesn't? Where is the difference?

@longlho
Copy link
Member

longlho commented Nov 4, 2020

defaultRichTextElements propagates to all messages while values only applies to that one message

@electerious
Copy link
Contributor

But there isn't a difference in performance? Applying the same values map to all FormattedMessages would be the same as using defaultRichTextElements, just without the warning?

@longlho
Copy link
Member

longlho commented Nov 4, 2020

That's now where the perf hit comes. If you manually apply those in messages that need values, yes perf is the same. The issue is since defaultRichTextElements is ambient we don't know if a message needs it or not without parsing. And parsing takes time

@electerious
Copy link
Contributor

Thanks for the clarification!

Porting a big project to the recommended way takes a while. It would therefore be great if the warning would be a warning instead of an error (console.warn instead of console.error). I can open a PR for this if that's something you agree with.

@longlho
Copy link
Member

longlho commented Nov 4, 2020

Yeah I'd be down for that

@salatielsql
Copy link

I'm not even using defaultRichTextElements but I'm getting hundreds of console.error saying:

[React Intl] "defaultRichTextElements" was specified but "message" was not pre-compiled. Please consider using "@formatjs/cli" to pre-compile your messages for performance.

My code:

      <IntlProvider locale={language} messages={messages(language)} defaultLocale="en">
         <App />
      </IntlProvider>

@longlho
Copy link
Member

longlho commented Nov 4, 2020

Do u have a repro?

@nicohagen
Copy link

Facing the same issue, just with warnings instead errors now, which are still spamming the console. Maybe IntlProvider can throw this warning once, but not for every single message?

@longlho
Copy link
Member

longlho commented Nov 19, 2020

Do you have a repro?

@nicohagen
Copy link

Do you have a repro?

The repro(duce?) is the same as the one in the original post. Add the defaultRichTextElements prop to <IntlProvider> while not using compiled messages and having a bunch of <FormatteMessage>.

@longlho
Copy link
Member

longlho commented Nov 19, 2020

Oh that's intentional. I thought you don't use it and the warnings still show up. What's stopping you from precompiling messages?

@nicohagen
Copy link

Nothing actually, I'd just prefer not to have the console spammed until it happened. A single notification about it would be enough as a reminder for this task. I take "Please consider using..." as a suggestion, but currently i feel kinda forced to do this asap to get my console back. Anyway, when it's intentional, so be it. Great feature btw! We still been on react-intl v2.4 so far and having proper richtext support now is awesome.

@longlho
Copy link
Member

longlho commented Nov 19, 2020

kk we can tweak the warning mechanism

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

No branches or pull requests

5 participants