-
Notifications
You must be signed in to change notification settings - Fork 378
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
RFC-001 - Macros #318
Merged
Merged
RFC-001 - Macros #318
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
cc944a7
docs: new version of rfc 001
tricoder42 3e66923
docs: add message descriptions to rfc
tricoder42 8a21e21
feat: add macros
tricoder42 d16135a
test: add example with macros
tricoder42 72ecce1
test: add vanillaJS example
tricoder42 3613c9b
test: add example of lazy translations
tricoder42 401569f
feat: add jsx macros
tricoder42 66568cd
test: pass plugin options if exists
tricoder42 1d84809
test: run vanilla-js integration test suit separately
tricoder42 e53ecd9
test: fix typo
tricoder42 921d5aa
test: install yalc packages after yarn
tricoder42 4897e64
test: make lingui executable
tricoder42 cf62467
fix: keep imports for extracting messages
tricoder42 0e08f9b
test: update webpack react example to use macros
tricoder42 7a4da55
test: remove obsolete example and make macros/babel 7 the default one
tricoder42 a6ddbde
test: run webpack example test suit separately
tricoder42 316cc0e
docs: update docs to use macros instead of plugins
tricoder42 7562f39
feat: update init command to install macros instead of plugins
tricoder42 bab02db
docs: update READMEs
tricoder42 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
42 changes: 21 additions & 21 deletions
42
examples/webpack-react-macros/src/Usecases/ElementAttributes.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,38 +1,38 @@ | ||
// @flow | ||
import * as React from "react" | ||
import { withI18n } from "@lingui/react" | ||
import { Trans } from "@lingui/react.macro" | ||
import { t } from "@lingui/js.macro" | ||
import { I18n } from "@lingui/react" | ||
import { t, Trans } from "@lingui/macro" | ||
|
||
type ElementAttributesProps = { | ||
i18n: Object | ||
} | ||
|
||
class ElementAttributes extends React.Component<ElementAttributesProps> { | ||
export default class ElementAttributes extends React.Component< | ||
ElementAttributesProps | ||
> { | ||
props: ElementAttributesProps | ||
|
||
render() { | ||
const { i18n } = this.props | ||
const articleName = "Scientific Journal" | ||
const closeLabel = t`Close` | ||
|
||
return ( | ||
<div> | ||
<Trans> | ||
<a | ||
className="expression" | ||
href="/article" | ||
title={t`Full content of ${articleName}`} | ||
> | ||
Article | ||
</a> | ||
</Trans> | ||
<button className="variable" aria-label={closeLabel}> | ||
X | ||
</button> | ||
</div> | ||
<I18n> | ||
{({ i18n }) => ( | ||
<div> | ||
<a | ||
className="expression" | ||
href="/article" | ||
title={i18n._(t`Full content of ${articleName}`)} | ||
> | ||
<Trans>Article</Trans> | ||
</a> | ||
<button className="variable" aria-label={i18n._(closeLabel)}> | ||
X | ||
</button> | ||
</div> | ||
)} | ||
</I18n> | ||
) | ||
} | ||
} | ||
|
||
export default withI18n()(ElementAttributes) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1,032 changes: 540 additions & 492 deletions
1,032
examples/webpack-react-macros/src/Usecases/__snapshots__/Children.test.js.snap
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it really an improvement considering how it was looking before? :) Was the previous approach working? I am not sure, haven't used that. Also, I think that in gitter we were talking about a variant like that which would be much nicer imo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean
i18n.t
overi18n._(t
? It's necessary to move to macros... I don't think there's a way to have a object property to be a macro.Other way would be convert it to
i18n._
directly, but for that we would needi18n
in scope, which wouldn't be used otherwise and linter would complain.This approach is very flexible, but the only drawback is how it looks :/ But you really get used to it. I wrote
vanilla-js
example and it was so great.i18n(t
is planned, I just have to figure out how to create callable object. I already started working on it but then figure out this is not the core of the problem and can be done later. Bothi18n._(..)
andi18n(...)
will be supported anyway until v3.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And yeah, the previous approach had several problems, especially when plugins were misconfigured. It doesn't allow you to use
this.props.i18n.t
, because onlyi18n.t
worked. You couldn't use it withi18n.use
method, which switches language locally (i18n.use('cs')._(t...)
). And also for lazy messages you had to use different API,i18nMark
. Now there's the same API for all messages, lazy/immediate translations, with/without IDs.As I said, I like everything about it except how it looks... but I believe it won't be such a problem once we have
i18n(t...)
API.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, a "callable object" is just a function with additional properties/methods attached directly to it. There is nothing wrong about that in my opinion and it will make it much easier to use and look at :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool! I believe there's nothing wrong with it as well, but it felt a bit hackish :) I'm halfway through with implementation, which I saved in different branch. I'll finalize it soon.