Skip to content
This repository has been archived by the owner on May 24, 2024. It is now read-only.

Improve usage of passed along react-intl modules #1982

Closed
bjankord opened this issue Oct 22, 2018 · 4 comments
Closed

Improve usage of passed along react-intl modules #1982

bjankord opened this issue Oct 22, 2018 · 4 comments

Comments

@bjankord
Copy link
Contributor

bjankord commented Oct 22, 2018

Feature Request

Description

We've started exposing react-intl modules from terra-base. One of the issues with this is related to how we expose the react-intl modules. We import modules from react intl and then export them from this file. This is then consumed like so: import { injectIntl, intlShape } from 'terra-base';.

This pulls in and executes more code than just injectIntl and intlShape, but it's not immediately obivous. This executes this file, which executes this line, which exectus this file from terra-i18n which executes our i18n logic.

If we want to continue exporting react-intl modules from terra-base, we should look at ways to do this that don't require the full terra-base and terra-i18n files to execute when importing the react-intl modules.

Additional Context / Screenshots

Consumers noticed this issue in a recent release of terra-application-header-layout where we added this line import { injectIntl, intlShape } from 'terra-base';.

@bjankord
Copy link
Contributor Author

bjankord commented Oct 23, 2018

Tech Design

Split react-intl into separate file exports under terra-base.

This allows terra-base to remain 1 dependency used to manage react-intl while allowing individual modules to be imported without pulling in all of terra-base.

  • Add peerDependency on react-intl to packages using react-intl. (This is similar to what we do with react, react-dom)
  • Remove passed through react-intl modules from terra-base and terra-i18n.

@bjankord bjankord self-assigned this Oct 23, 2018
@bjankord
Copy link
Contributor Author

Discussed with @tbiethman about this issue. We think a better path forward would be to avoid passing react-intl modules through terra-i18n / terra-base and instead manage the version of react-intl through peerDependencies in components that need intl similar to how we manage the version of react that components use.

With this, each package that needs modules from react-intl would import directly from react-intl and set a peerDependency on react-intl. The react-intl modules that are passed through terra-i18n and terra-base should be removed in a major version bump.

@emilyrohrbough
Copy link
Contributor

+1 on peer-dependencies.

@noahbenham
Copy link
Contributor

noahbenham commented Oct 25, 2018

react-intl as a peer dep would also allow consumers to begin using the new context API sooner once this PR merges.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants