Skip to content

Export all from TranslationContext file - #2829

Closed
maoueh wants to merge 1 commit into
marmelab:masterfrom
maoueh:fix/export-translation-context
Closed

maoueh wants to merge 1 commit into
marmelab:masterfrom
maoueh:fix/export-translation-context

Conversation

@maoueh

@maoueh maoueh commented Jan 29, 2019

Copy link
Copy Markdown
Contributor

The TranslationContext instance is required so a component can be flagged with the correct static contextType = TranslationContext value. Without setting that (at least in React 16.7), the this.context value is fully undefined and the component is not actually tied to the context.

By exporting it, consumer can simply set the context type to the value of import { TranslationContext } from react-admin` and everything works out of the box.

The `TranslationContext` instance is required so a component can be flagged
with the correct `static contextType = TranslationContext` value. Without
setting that (at least in React 16.7), the `this.context` value is fully
undefined and the component is not actually tied to the context.

By exporting it, consumer can simply set the context type and everything
works out of the box.
@fzaninotto

Copy link
Copy Markdown
Member

I'm not sure I understand, why don't you use the TranslationProvider?

@maoueh

maoueh commented Jan 30, 2019

Copy link
Copy Markdown
Contributor Author

So, I was following along https://marmelab.com/react-admin/Translation.html (Translating Your Own Components) which states that this.context can be used.

However, I wasn't able to make it work with:

render() {
  console.log(this.context) // Prints `undefined`
}

I had to do this instead to make it work:

static contextType = TranslationContext // Imported using `import { TranslationContext } from "ra-core/lib/i18n/TranslationContext"` 

render() {
   console.log(this.context) // Now prints correctly `{ t: function (...), locale: "en" }`
}

Don't hesitate to point to some misconceptions about this usage.

@djhi

djhi commented Jan 30, 2019

Copy link
Copy Markdown
Contributor

@maoueh You should read the whole section, it also specifies that you should use the translate HOC. However, as this could be confusing, we should probably remove the part about context altogether

@maoueh

maoueh commented Jan 30, 2019

Copy link
Copy Markdown
Contributor Author

I read everything, and using the HOC works great, I wanted to test out this.context since it allows for much less boilerplate IMO.

Is the context not supported anymore? Would be sad :) But if it's indeed deprecated, I can re-write this part of the docs.

Even after using static contextType, it seems it's not working as expected, translations are not found, I need to investigate further (it it's a supported feature :D).

Simply tells me you would prefer between ditching it or supporting it.

@djhi

djhi commented Jan 30, 2019

Copy link
Copy Markdown
Contributor

I'd rather have only one way to do something so I would recommend ditching the context documentation. Ping @fzaninotto

@fzaninotto

Copy link
Copy Markdown
Member

I agree, we should remove the part about the context in the documentation. Using the translate HOC, we can abstract the actual implementation (and it will make it easier to transition to React hooks in the near future).

@maoueh Would you be willing to do a PR on the documentation for that?

@maoueh

maoueh commented Jan 31, 2019

Copy link
Copy Markdown
Contributor Author

Replaced by #2841

@maoueh maoueh closed this Jan 31, 2019
@maoueh
maoueh deleted the fix/export-translation-context branch January 31, 2019 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants