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

[RFR] Bootstrap TypeScript migration #2426

Merged
merged 15 commits into from
Oct 17, 2018
Merged

[RFR] Bootstrap TypeScript migration #2426

merged 15 commits into from
Oct 17, 2018

Conversation

fzaninotto
Copy link
Member

@fzaninotto fzaninotto commented Oct 12, 2018

We've decided to gradually migrate react-admin to TypeScript. Explaining why isn't the purpose of this description, but we expect more stability and a better developper experience.

Our strategy is:

  • Switch transpiler from Babel to TypeScript (both for CJS and ESM builds)
  • Change the minimum code in the packages to make it work (mostly export *)
  • Change one file from the core to TypeScript to make sure the whole chain works
  • Check that the simple example still works
  • Migrate the simple example to Babel 7
  • Replace ESLint by TSLint
  • Check that the demo example still works
  • Check that tests still pass
  • Document the new standard(it will only be a standard for packages we've migrated)

Then, we'll rename files from .js to .ts/.tsx and add types little by little (outside of the scope of this PR)

Comments are welcome!

Copy link
Collaborator

@djhi djhi left a comment

Choose a reason for hiding this comment

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

👍

tsconfig.json Outdated
"compilerOptions": {
/* Basic Options */
"target":
"ES3" /* Specify ECMAScript target version: 'ES3' (default), 'ES5', 'ES2015', 'ES2016', 'ES2017','ES2018' or 'ESNEXT'. */,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Es3 ?

Copy link

Choose a reason for hiding this comment

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

In fact there is no compilation done by type script, just typechecking babel still does the job with ts-preset

Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't it tsc which is used to build now according to the package.json files ?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes indeed

Copy link

@brikou brikou left a comment

Choose a reason for hiding this comment

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

Great 1st step!

tsconfig.json Outdated
"compilerOptions": {
/* Basic Options */
"target":
"ES3" /* Specify ECMAScript target version: 'ES3' (default), 'ES5', 'ES2015', 'ES2016', 'ES2017','ES2018' or 'ESNEXT'. */,
Copy link

Choose a reason for hiding this comment

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

In fact there is no compilation done by type script, just typechecking babel still does the job with ts-preset

tsconfig.json Outdated
// "isolatedModules": true, /* Transpile each file as a separate module (similar to 'ts.transpileModule'). */

/* Strict Type-Checking Options */
"strict": true /* Enable all strict type-checking options. */,
Copy link

Choose a reason for hiding this comment

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

Should be commented for now

"allowJs": true /* Allow javascript files to be compiled. */,
// "checkJs": true, /* Report errors in .js files. */
"jsx":
"react" /* Specify JSX code generation: 'preserve', 'react-native', or 'react'. */,
Copy link

Choose a reason for hiding this comment

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

Should be preserve, otherwise some babel plugins may not work anymore

Copy link

Choose a reason for hiding this comment

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

forget about that, because of --noEmit :)

// "rootDir": "./", /* Specify the root directory of input files. Use to control the output directory structure with --outDir. */
// "composite": true, /* Enable project compilation */
// "removeComments": true, /* Do not emit comments to output. */
// "noEmit": true, /* Do not emit outputs. */
Copy link

Choose a reason for hiding this comment

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

Should be noEmit, ... Babel does the emit job

@fzaninotto
Copy link
Member Author

@brikou we use babel to test react-admin with the simple example, but we'll use Typescript to compile the ES6/TS code to ES5 for packaging, so we need both the jsx:react and the emit.

@eknowles
Copy link

Great plan! I'd love to contribute to this migration

@fzaninotto
Copy link
Member Author

@eknowles you could help by explaining me why the unit tests fail... Apparently, transpiling js components with TypeScript messes up some of the component displayName, which appears as 'Component rather than e.g. ChipInput or ArrayField

@fzaninotto fzaninotto changed the title [WIP] Bootstrap TypeScript migration [RFR] Bootstrap TypeScript migration Oct 17, 2018
@fzaninotto
Copy link
Member Author

Green, and Ready for Review

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.

4 participants