Generate ID's for <FormattedMessage> components #31

bradbarrow · 2018-02-06T13:36:12Z

What:

Closes #27

babel-plugin-react-intl-auto will generate a globally unique ID for <FormattedMessage> components that don't specify one themselves.

Why:

react-intl provides two primary interfaces for defining and rendering formatted strings: the defineMessages function and <FormattedMessage> React component.

Currently, this plugin only supports generating an ID for messages defined via defineMessages.

We would like to be able to use the React component but still don't want to have to deal with unique IDs.

How:

This plugin (essentially) creates ID's for defineMessages by combining the key passed to defineMessages with the path to the current file.

With <FormattedMessage> we don't have any key at all, just the message (descriptions are optional).

So this PR proposes a way of creating ID's for React components by combining the path to the current file with a hash of the provided defaultMessage.

This ensures that per file, all occurrences of a given defaultMessage will have the same unique ID. You will never get two matching ID's with different messages.

Note We considered including per file instance variance in the ID (line number of the component for e.g) but decided this could cause a lot of churn in the ID as components are moved in a file.

Checklist:

Documentation
Tests
Ready to be merged
Added myself to contributors table

bradbarrow · 2018-02-08T00:41:47Z

Sorry, we PR'd this early by mistake. Still a WIP but we intend to propose the change.

bradbarrow · 2018-02-08T13:30:08Z

Hi, we think this is ready to go - would love to hear what you think :)

akameco · 2018-02-08T15:06:02Z

@bradbarrow Great work🎉
I'll merge it after review 👍

akameco · 2018-02-08T15:07:59Z

src/__tests__/components.test.js

+    title: '',
+    plugin,
+    snapshot: true,
+    babelOptions: { filename, babelrc: true },


use babelOptions: { filename, parserOpts: { plugins: ['jsx'] } },
remove babelrc: true

Oh cool... I understand the dependency removal now too thanks. What does it use to parse jsx under the hood?

See https://github.com/babel/babel/blob/master/packages/babylon/README.md

akameco · 2018-02-08T15:08:42Z

src/index.js

@@ -1,10 +1,15 @@
 // @flow
 import p from 'path'
 import * as t from 'babel-types'
+import murmur from 'murmurhash3js'


Why is this library?

What: an efficient hashing library to create unique but short hashes

Why: wanted to use the defaultMessage as part of the ID but they are difficult to sanitize and might be quite long. This is the same hash used in the Babel react intl hash plugin

akameco · 2018-02-08T15:09:35Z

src/index.js

@@ -16,7 +21,7 @@ const isImportLocalName = (name: string, { file }: State) => {
        for (const specifier of path.get('specifiers')) {
          if (
            specifier.isImportSpecifier() &&
-            specifier.node.imported.name === 'defineMessages'
+            allowedNames.includes(specifier.node.imported.name)


akameco · 2018-02-08T15:10:56Z

package.json

@@ -64,6 +65,7 @@
    "babel-plugin-tester": "^5.0.0",
    "babel-preset-env": "^1.6.1",
    "babel-preset-flow": "^6.23.0",
+    "babel-preset-react": "^6.24.1",


I found that without this, jest was unable to run my component fixtures. It throws errors when it finds the JSX nodes

akameco · 2018-02-08T15:12:54Z

src/__tests__/components.test.js

+  code: `
+import { FormattedMessage } from 'react-intl'
+
+console.log(<FormattedMessage defaultMessage="hello" />);


Why need console.log ?

It doesn't. I wanted to use the smallest possible valid code sample.i could just have the component by itself...It just felt like it would be floating...And not a typical use case 😄 happy to remove since I suppose Babel doesn't care

I tried to remove this, and in fact it does throw an error when <FormattedMessage> is just in line on it's own. So, I've left the log to give it some structure.

Open to other ideas like a function:

const Component = () => <FormattedMessage />

or something?

akameco · 2018-02-08T15:14:06Z

src/index.js

+    // Evaluate the message expression to see if it yields a string
+    const evaluated = messageValuePath.get('expression').evaluate()
+
+    //if (evaluated.confident && t.isStringLiteral(evaluated)) {


remove this comment.

Oops sorry :)

akameco · 2018-02-08T15:20:13Z

src/__tests__/components.test.js

+  code: `
+import { FormattedMessage } from 'react-intl'
+
+console.log(<FormattedMessage defaultMessage={\`hello world ${1}\`} />);


clear intention 1 →1 + 1

Hi I'll change that here and in the other test too

akameco · 2018-02-08T15:30:02Z

And please explain the description to the readme :)

bradbarrow · 2018-02-08T21:30:22Z

Hi I'll sort out those changes today :)

I'm not sure what you'd like me to explain about the Readme?

I tried to add a few examples and additions to show how this component based approach would look and what the result is.

I added some minor formatting changes to make the section about extracting a little clearer ... I hope.

bradbarrow · 2018-02-08T22:59:42Z

Ok feedback addressed. I have left the console logs as it doesn't work without some kind of wrapper for context.

bradbarrow · 2018-02-09T12:27:35Z

That worked, changes pushed @akameco thank you :)

Generates and inserts globally unique id props for react-intl React components FormattedMessage and FormattedHTMLMessage

akameco · 2018-02-12T00:22:44Z

src/__tests__/index.test.js

@@ -74,7 +74,7 @@ const tests = [
 import { defineMessages } from 'react-intl'

 defineMessages({
-  hello: \`hello world \${1}\`,
+  hello: \`hello world \${1+1}\`,


Do not change. Because no need to evaluate here.

akameco · 2018-02-12T00:23:10Z

Do not rebase😱 I'd like to see diff because of the cost of reviews.

bradbarrow · 2018-02-12T00:36:54Z

Fixed up the test :) Sorry, rebasing is my default workflow. Have added the latest fix as a new commit

bradbarrow · 2018-02-12T04:10:11Z

Thanks for the review @akameco :) We're looking forward to the publish so we can start using this in production - we'll be sure to raise any improvements once we've used it for a while :)

Thanks for being available 👍

bradbarrow · 2018-02-13T12:04:26Z

Hi @akameco, just wondering if you have an ETA on this release? We're relying on it for a piece of work and we'd love to deliver it this week :)

Thank you 🙏

akameco · 2018-02-13T13:00:42Z

@bradbarrow I released 1.1.0 🎉 Great work 👍

bradbarrow changed the title ~~WIP~~ Generate ID's for FormattedMessages Feb 6, 2018

akameco self-requested a review February 6, 2018 15:28

akameco added the enhancement label Feb 6, 2018

bradbarrow force-pushed the generate-id-for-formatted-message branch 3 times, most recently from 5295977 to f96f4cb Compare February 8, 2018 13:13

bradbarrow changed the title ~~Generate ID's for FormattedMessages~~ Generate ID's for <FormattedMessage> components Feb 8, 2018

bradbarrow force-pushed the generate-id-for-formatted-message branch from f96f4cb to 8f9e880 Compare February 8, 2018 13:29

akameco requested changes Feb 8, 2018

View reviewed changes

bradbarrow force-pushed the generate-id-for-formatted-message branch from 8f9e880 to 504c729 Compare February 8, 2018 22:55

bradbarrow force-pushed the generate-id-for-formatted-message branch from 504c729 to f6416cc Compare February 9, 2018 12:27

feat: auto ID's for react-intl components

6a25249

Generates and inserts globally unique id props for react-intl React components FormattedMessage and FormattedHTMLMessage

bradbarrow force-pushed the generate-id-for-formatted-message branch from f6416cc to 6a25249 Compare February 9, 2018 12:29

akameco requested changes Feb 12, 2018

View reviewed changes

bradbarrow force-pushed the generate-id-for-formatted-message branch from 4c5f4f4 to 248a9e5 Compare February 12, 2018 01:00

No need to evaluate expression in defineMessages test

37b553b

bradbarrow force-pushed the generate-id-for-formatted-message branch from 248a9e5 to 37b553b Compare February 12, 2018 01:01

akameco self-requested a review February 12, 2018 02:21

akameco approved these changes Feb 12, 2018

View reviewed changes

akameco merged commit df2fdcc into akameco:master Feb 13, 2018

akameco mentioned this pull request Aug 23, 2018

Support for <FormattedMessage /> API #44

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generate ID's for <FormattedMessage> components #31

Generate ID's for <FormattedMessage> components #31

bradbarrow commented Feb 6, 2018 •

edited

bradbarrow commented Feb 8, 2018

bradbarrow commented Feb 8, 2018

akameco commented Feb 8, 2018

akameco Feb 8, 2018

bradbarrow Feb 8, 2018

akameco Feb 9, 2018

akameco Feb 8, 2018

bradbarrow Feb 8, 2018

akameco Feb 9, 2018

akameco Feb 8, 2018

akameco Feb 8, 2018

bradbarrow Feb 8, 2018

akameco Feb 8, 2018

bradbarrow Feb 8, 2018

bradbarrow Feb 8, 2018

akameco Feb 8, 2018

bradbarrow Feb 8, 2018

akameco Feb 8, 2018

bradbarrow Feb 8, 2018

akameco commented Feb 8, 2018

bradbarrow commented Feb 8, 2018

bradbarrow commented Feb 8, 2018

bradbarrow commented Feb 9, 2018

akameco Feb 12, 2018

bradbarrow Feb 12, 2018

akameco commented Feb 12, 2018

bradbarrow commented Feb 12, 2018

bradbarrow commented Feb 12, 2018

bradbarrow commented Feb 13, 2018

akameco commented Feb 13, 2018

Generate ID's for <FormattedMessage> components #31

Generate ID's for <FormattedMessage> components #31

Conversation

bradbarrow commented Feb 6, 2018 • edited

bradbarrow commented Feb 8, 2018

bradbarrow commented Feb 8, 2018

akameco commented Feb 8, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akameco commented Feb 8, 2018

bradbarrow commented Feb 8, 2018

bradbarrow commented Feb 8, 2018

bradbarrow commented Feb 9, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akameco commented Feb 12, 2018

bradbarrow commented Feb 12, 2018

bradbarrow commented Feb 12, 2018

bradbarrow commented Feb 13, 2018

akameco commented Feb 13, 2018

bradbarrow commented Feb 6, 2018 •

edited