Add babel macros as a replacement for transform plugins

[mlabrum]

Hey,

I've written an babel-plugin-macros implementation of @lingui/babel-plugin-transform-react based on this issue #86 I'm opening this PR for early feedback to ensure this is the direction that you wanted to go in :)

I've taken a few shortcuts as I figured that we would want to support both the macros and babel-plugin-transform-react implementations, so macros currently calls the transformer within babel-plugin-transform-react so there's only one implementation of the transformer.

Example from one of the test files....

import { Trans } from '@lingui/react-macro';

<span>Without translation</span>;
<Trans>Hello World</Trans>;
<Trans id="Hello World">Hello World</Trans>;
<Trans id="msg.hello">Hello World</Trans>;
<Trans id={msg} />;

to

import { Trans } from "@lingui/react";
<span>Without translation</span>;
<Trans id="Hello World" />;
<Trans id="Hello World" />;
<Trans id="msg.hello" defaults="Hello World" />;
<Trans id={msg} />;

TODO:

• Refactor babel-plugin-transform-react so the transformer doesn't depend on Import Declarations so I don't need setImportDeclarations method... which breaks the babel plugin (and fails the tests :))
• Add @lingui/react-macro to babel-plugin-lingui-extract-messages package detection
• Change babel-plugin-transform-react to use ES6 modules again ( I changed this when I had some errors in macro, I should be able to revert this change as @lingui has a buildstep to compile this out )
• Add react-macro to build bundles
• Implement macro for babel-plugin-transform-js
• Figure out how to ensure intellisense works https://github.com/lingui/js-lingui/pull/193/files#diff-13b65913d74e56772a41fd137d19a6a6R66 maybe a peer dependency on @lingui-react and copy the proptypes over

Cheers,

[tricoder42]

This is amazing! I'm going to review it asap

[mlabrum]

I've just got back from holiday, so let me know if you're fine with how I've done the first macro and I'll implement the next part ( js macro))

…

On Wed, 18 Apr. 2018, 7:15 am Tomáš Ehrlich, ***@***.***> wrote: This is amazing! I'm going to review it asap — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#193 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AASEqhIltieYnJcBvUiR6jZ_GuRW2PAAks5tpmJcgaJpZM4SvcEs> .

[mlabrum]

@tricoder42 any comments?

[tricoder42]

Hey @mlabrum, I can't believe I left this PR without attention for so long. This is awesome! Are you still interested in writing js macro?

I'm thinking that this method should be the default one. After all, there's a lot of confusion how i18n components and methods work and it would be easier to comprehend if all these helpers were macros

• it would send a clear message that macros are optional and it's possible to use jsLingui without them (although with disadvantage of writing messages by hand)
• it would be more clear what happens under the hood (e.g. everything wrapped in macro is processed and replaced at compile time).

Let me know if I can help with anything and please accept my apology for such a late response

[tricoder42]

@mlabrum I merged stable-2.x branch, resolved conflicts and fixed tests. I also renamed package to @lingui/react.macro and now I'm going to copy example project and migrate it to macros. I have a feeling that all I have to do is rename imports...

This is pretty good, thank you again!

I would like to release it soon and we can work on js.macro later. We have to figure out how to pass global i18n object to macros js.macro. In react.macro we simply transform macro -> component, like Trans -> Trans or Plural -> Trans, but in js.macro we need t -> i18n.t or plural -> i18n.plural 🤔 But that's another story, this macro is sweat and ready for release.

[codecov]

Codecov Report

Merging #193 into stable-2.x will increase coverage by 0.1%.
The diff coverage is 98.37%.

@@              Coverage Diff              @@
##           stable-2.x     #193     +/-   ##
=============================================
+ Coverage       91.39%   91.49%   +0.1%     
=============================================
  Files              41       49      +8     
  Lines             953     1023     +70     
=============================================
+ Hits              871      936     +65     
- Misses             82       87      +5

Impacted Files	Coverage Δ
packages/babel-plugin-transform-react/src/index.js	100% <100%> (ø)	⬆️
...es/babel-plugin-transform-react/src/transformer.js	100% <100%> (ø)
packages/react.macro/index.js	100% <100%> (ø)
...ackages/babel-plugin-extract-messages/src/index.js	96.93% <100%> (ø)	⬆️
packages/babel-plugin-transform-react/index.js	100% <100%> (ø)
...ckages/babel-plugin-transform-react/transformer.js	100% <100%> (ø)
packages/babel-plugin-transform-js/src/index.js	100% <100%> (+0.75%)	⬆️
packages/js.macro/index.js	100% <100%> (ø)
packages/react.macro/src/index.js	85.71% <85.71%> (ø)
packages/js.macro/src/index.js	93.75% <93.75%> (ø)
... and 9 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1be97cb...f3c5870. Read the comment docs.

[tricoder42]

React macros looks solid. There's an example with integration tests and everything works fine.

Thinking about js.macro, the main caveat is i18n object. We basically need a macro t which is transformed into i18n._, where i18n is an object in scope.

Option 1: i18n in scope (hackish)

If we simply assume, there's an i18n object in scope and let user to manager it, then it's easy but linters will complain about unused variable:

const i18n = setupI18n()

t`Hello ${name}`

// becomes
// i18n._("Hello {name}", { name })

Option 2: Macro wrapped in i18n._ (ugly)

Another option is more explicit - pass output of macro to i18n._ call:

const i18n = setupI18n()

i18n._(t`Hello ${name}`)

// should become
// i18n._("Hello {name}", { name })

First, I'm not sure if it's even possible. We're taking a single AST node and returning a list. It may also be a bit confusing, but after all, this is exactly what js.macro is doing: transforming template literals into string (and also variables).

Option 3: Object member as a macro? (ideal)

This would be ideal, but don't think it's possible

import { t } from '@lingui/js.macro` // linter still complains here

const i18n = setupI18n()

// t is a macro here
i18n.t`Hello ${name}`

// becomes
// i18n._("Hello {name}", { name })

but still we need to cover another usecase - custom message ID:

i18n('msg.id')`Hello ${name}`

but that could be e.g.:

i18n.id('msg.id')`Hello ${name}`

where id is macro.

[tricoder42]

js.macro is ready. I chose the first option and macro is checking if i18n is in scope. Otherwise it warns user with explanation. It's not ideal for linter, but it's definitely better than other options (like i18n(t'...') or i18n._(t'...')

Final step is to add "dummy" components/methods with proptypes/flowtypes so the auto-completition works correctly even for macros.

[tricoder42]

Alright, merging now and I'll fix flow types later. I need to fix #250 first anyway…

Thank you @mlabrum for working on this! You really did an awesome job here!