Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

chore(docs): use babel-loader #1354

Merged
merged 10 commits into from May 22, 2019
Merged

chore(docs): use babel-loader #1354

merged 10 commits into from May 22, 2019

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented May 17, 2019

Related #508.

Enable isolatedModules in TS compilerOptions

Babel behaves as if the --isolatedModules option was passed to the TypeScript Compiler and this can't be worked around because Babel doesn't support cross-file analysis.

How does Babel handle TypeScript code? It removes it, it strips out all the TypeScript, turns it into "regular" JavaScript. Any bundler don't know anything about types because don't exists in runtime and that's why types can't be reexported.

https://babeljs.io/docs/en/babel-plugin-transform-typescript

Updates in exports

As mentioned before we can't reexport types, that's why exports were transformed:

Before

export { default as Accordion, AccordionProps } from './Accordion'

After

export { default as Accordion } from './Accordion'
export * from './Accordion' // will AccordionProps and everything except default

types.internal.ts

These files will contain shared and reusable types in package, but these types shouldn't be part of a public API and can't be exported.

export * from './types' // contains only types that are part of public API

Remove awesome-typescript-loader & ts-loader

Was replaced with babel-loader and fork-ts-checker-webpack-plugin to keep type checks.

Add fork-ts-checker-webpack-plugin

Webpack plugin that runs TypeScript type checker on a separate process. It allows very fast compile code by babel-loader and run type checks parallel, so by switching to Babel we will not loose TS checks.


This PR do not resolve issues with HMR.

@layershifter layershifter marked this pull request as ready for review May 21, 2019 13:57
@layershifter layershifter changed the title Chore/babel loader chore(docs): use babel-loader May 21, 2019
@codecov
Copy link

codecov bot commented May 21, 2019

Codecov Report

Merging #1354 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1354   +/-   ##
=======================================
  Coverage   72.92%   72.92%           
=======================================
  Files         775      775           
  Lines        5824     5824           
  Branches     1721     1698   -23     
=======================================
  Hits         4247     4247           
  Misses       1571     1571           
  Partials        6        6
Impacted Files Coverage Δ
...es/react/src/components/Popup/positioningHelper.ts 100% <ø> (ø) ⬆️
packages/react/src/index.ts 100% <ø> (ø) ⬆️
...ckages/react-component-event-listener/src/index.ts 100% <ø> (ø) ⬆️
...mponent-event-listener/src/lib/addEventListener.ts 87.5% <ø> (ø) ⬆️
...eact-component-event-listener/src/EventListener.ts 100% <ø> (ø) ⬆️
packages/react/src/types.ts 50% <ø> (ø) ⬆️
...eact/src/themes/teams/components/Icon/svg/index.ts 25% <ø> (ø) ⬆️
...onent-event-listener/src/StackableEventListener.ts 61.11% <ø> (ø) ⬆️
...nent-event-listener/src/lib/removeEventListener.ts 88.88% <ø> (ø) ⬆️
...act-component-event-listener/src/types.internal.ts 100% <100%> (ø)
... and 1 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 63cf45c...0f76f2d. Read the comment docs.

@@ -1,6 +1,7 @@
{
"extends": "./tsconfig.common.json",
"compilerOptions": {
"isolatedModules": false,
Copy link
Contributor

@kuzhelov kuzhelov May 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this config and the next one are used for yarn build scenarios, where Babel is not used.

This is necessary to ensure that type declarations will be generated.

@@ -1 +1,2 @@
export { default, ComponentExampleProps } from './ComponentExample'
export * from './ComponentExample'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these changes in exports are necessary to make TS with --isolatedModules flag work - this flag ensures that TS compiler uses the same 'file-by-file' processing strategy as Babel

@@ -8,13 +8,8 @@ export type Extendable<T, V = any> = T & {
[key: string]: V
}

export type Nullable<T> = T | null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all these are unused, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly 😇

@@ -1,3 +1,5 @@
import '@babel/polyfill'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you, please, suggest why this change was necessary? where it is used?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It contains regeneratorRuntime that it's required by async/await transform.

@kuzhelov
Copy link
Contributor

just couple of moments that we might add to the description (all those are mentioned in the comments):

  • brief explanation on how transpilation logic has changed - from recursive modules resolution used by Typescript to isolated files/modules used by Babel. This will serve as concise reasoning behind --isolatedModules flag that is introduced to TS compiler's config
  • provide brief explanation on why export statements need rework
  • provide explanation on the strategy with types.private (mb types.internal, just to better reflect semantics?) modules - whey these are necessary and which problem they help to address

@DustyTheBot
Copy link
Collaborator

Warnings
⚠️ There are no updates provided to CHANGELOG. Ensure there are no publicly visible changes introduced by this PR.

Generated by 🚫 dangerJS

@layershifter layershifter merged commit 3f33137 into master May 22, 2019
@delete-merged-branch delete-merged-branch bot deleted the chore/babel-loader branch May 22, 2019 20:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants