Skip to content
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

Typescript use export default for esm-first approach #1352

Merged
merged 2 commits into from Oct 29, 2019

Conversation

rosskevin
Copy link
Collaborator

@rosskevin rosskevin commented Oct 29, 2019

Rationale

Long history, explanation and rationale in issue #1351. The runtime and pure javascript remains unaffected.

Moving forward

Use the following import styles regardless of the esModuleInterop setting (sample imports of default and named below, choose the one that fits your use case):

  • import i18next from 'i18next'
  • import {TFunction} from 'i18next'
  • import i18next, {TFunction} from 'i18next'

Implications for esModuleInterop: true users

NO CHANGE at all! Yay.

Implications for esModuleInterop: false (default) users

This is a sample change set for someone that uses both a default and a named import. Adjust imports to your use case accordingly as mentioned above.

-`import * as i18next from 'i18next'`
- const {TFunction} = i18next
+ `import i18next, {TFunction} from 'i18next'`
  1. jest breaks - it is run with node and there is no esm loading in ts-jest (they recommend running with esModuleInterop) or with @std/esm (open issue about jest support)
  2. ts-node breaks under certain settings

These breakages are expected and intended.

It ultimately comes down to this: i18next is just an ecmascript library.

Loading semantics of webpack, node, jest and other tools are not in the purview of this project. This change recognizes that understanding and aligns the typescript types exactly with the source code and we are using an esm-first approach moving forward.


I ask that before registering a dissenting comment here, please look at the linked issue, repository with reproductions and all the research I documented over the past eight hours of work. This is a seemingly innocuous change that will affect some typescript users, but it has been thoroughly researched and it is in-line with typescript recommendations.

Closes #1351

@coveralls
Copy link

coveralls commented Oct 29, 2019

Coverage Status

Coverage remained the same at 93.305% when pulling 13b10da on rosskevin:ts-export-default into 7671a5a on i18next:master.

rosskevin added a commit to rosskevin/test-i18next that referenced this pull request Oct 29, 2019
@rosskevin rosskevin merged commit b6f9fdb into i18next:master Oct 29, 2019
@rosskevin
Copy link
Collaborator Author

rosskevin commented Oct 29, 2019

@jamuhl this is a typescript-only breaking change. It is your call on what to do with semver here. I can argue for a major or minor bump. If I were persuaded in one direction, on this case it would be major.

I will be following on with PRs for other i18next projects to deal with the ts interop tests/imports.

@rosskevin rosskevin deleted the ts-export-default branch Oct 29, 2019
rosskevin added a commit to rosskevin/react-i18next that referenced this pull request Oct 29, 2019
rosskevin added a commit to rosskevin/i18next-browser-languageDetector that referenced this pull request Oct 29, 2019
rosskevin added a commit to rosskevin/i18next-xhr-backend that referenced this pull request Oct 29, 2019
@jamuhl
Copy link
Member

jamuhl commented Oct 29, 2019

@rosskevin prefer major too...give me a few minutes to publish this.

@jamuhl
Copy link
Member

jamuhl commented Oct 29, 2019

published in i18next@19.0.0

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

Successfully merging this pull request may close these issues.

Reconsider typescript export to cover both esModuleInterop modes and esm by default
3 participants