Skip to content
This repository has been archived by the owner on Jun 8, 2019. It is now read-only.

Add support for processing intl.formatMessage when injection is used. #165

Merged
merged 3 commits into from
May 22, 2019

Conversation

swernerx
Copy link
Contributor

In our project we had the need to extract from intl.formatMessage as well. According to your documentation this is a feature you do not want to include by default. I perfectly understand the reasoning that it is hard, by any system, to automatically check whether it's the formatMessage injected by react-intl.

In our case we would prefer using formatMessage and life with the risk that some calls might be wrongly analyzed. We just follow the convention that to make it parsable the formatMessage() call has to be called in the intl object. In our application we feel this limitation is pretty safe - and might be for a lot of other uses.

I opened this PR for our team to use and for a general discussion on the topic. Probably we can add a parameter to the plugin to enable this non-safe extraction as well.

What do you think?

@kasparasg
Copy link

I'd love to see this merged 👍

Copy link

@SavageWilliam SavageWilliam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also need this!

@atomcorp
Copy link

atomcorp commented May 3, 2019

Thanks @swernerx! This works a treat!

To the maintainers, our app had something like 1,500 formatMessages, the prospect of converting them to use defineMessage was not appealing. As this person pointed out in 2016 this should be really obvious in the docs. Although preferably this pull request would be merged!

@longlho
Copy link
Member

longlho commented May 21, 2019

Ack. will review asap

@macca16
Copy link

macca16 commented May 27, 2019

I see this was committed in formatjs/formatjs@b57953e by @longlho

Unfortunately with it being enabled by default it is causing extraction failures in my usage of this.props.intl.formatMessage() as I have numerous usages where I defined localisations e.g.

const translations = defineMessages({
    mystring: {
        id: 'keyname',
        description: 'my desc',
        defaultMessage: 'my english',
    },
});

and used as follows:
this.props.intl.formatMessage(translations.mystring)

Without the suggested option to make this change opt in and no obvious way to opt out of this functionality I needed for now to downgrade babel-plugin-react-intl to 3.0.1

@longlho
Copy link
Member

longlho commented May 27, 2019

@macca16 can u create an issue in formatjs/formatjs? THanks!

larskarlitski added a commit to larskarlitski/cockpit-composer that referenced this pull request May 27, 2019
3.0.2 is broken for us because of

    formatjs/babel-plugin-react-intl#165

`formatMessafe()` calls are not recognized as coming from injectIntl().
larskarlitski added a commit to larskarlitski/cockpit-composer that referenced this pull request May 27, 2019
3.0.2 is broken for us because of

    formatjs/babel-plugin-react-intl#165

`formatMessage()` calls are not recognized as coming from injectIntl().
martinpitt pushed a commit to osbuild/cockpit-composer that referenced this pull request May 27, 2019
3.0.2 is broken for us because of

    formatjs/babel-plugin-react-intl#165

`formatMessage()` calls are not recognized as coming from injectIntl().
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants