Add TypeScript support #5027
Merged
Add TypeScript support #5027
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ample components to typescript and add an example definition file.
…rror while parsing tsconfig.json, The 'files' list in config file 'tsconfig.json' is empty` See (TypeStrong/ts-loader#405 (comment))
…ker container (only webpack.config.js is copied over from root folder in Dockerfile)
webpack.config.js
Outdated
| const tsConfigPath = path.join(basePath, "tsconfig.json"); | ||
|
|
||
| if (!fs.existsSync(tsConfigPath)) { | ||
| throw new Error(`Can not find tsconfig: ${tsConfigPath}`); |
There was a problem hiding this comment.
I added this line because the initial error I got for the missing tsConfig file in the docker container was missleading (see TypeStrong/ts-loader#405 (comment))
I then figured out that I either need to move tsconfig into /client or copy it over in the Dockerfile.
benamorbe
reviewed
Jul 9, 2020
client/tsconfig.json
Outdated
| "target": "es5", | ||
| "jsx": "react", | ||
| "allowJs": true | ||
| } |
There was a problem hiding this comment.
What about also adding allowSyntheticDefaultImports to have similar imports as in js files?
Without that option we have to import like this
import * as React from 'react';With this option ON we can import AND destructure like this:
import React, { useState, useEffect, Component } from 'react';
client/jsconfig.json
Outdated
| } |
There was a problem hiding this comment.
I think jsconfig.json is ignored/skipped when a tsconfig.json file exists too. Also having 2 files makes syncing difficult between both. In that case we can have both "allowJS": true and "checkJS": true in tsconfig.json.
| "jsx": "react", | ||
| "allowJs": true | ||
| } | ||
| } |
There was a problem hiding this comment.
Can you add explicit include to avoid compiling non-source files? Something like:
Suggested change
| } | |
| "include": [ | |
| "app" | |
| ] | |
| } |
client/tsconfig.json
Outdated
| "module": "es6", | ||
| "target": "es5", | ||
| "jsx": "react", | ||
| "allowJs": true |
There was a problem hiding this comment.
Can you also add DOM/ES libs?
Suggested change
| "allowJs": true | |
| "allowJs": true, | |
| "lib": [ | |
| "dom", | |
| "dom.iterable", | |
| "es6" | |
| ], |
client/tsconfig.json
Outdated
| "target": "es5", | ||
| "jsx": "react", | ||
| "allowJs": true | ||
| } |
There was a problem hiding this comment.
Other useful/recommended compiler options :
skipLibCheck: true: skips unnecessary lib checks in*.d.tsfiles (usually outside our scope)noEmit: true: We usewebpackso we should avoid emitting any output accidentally when runningtscmanually.strict: true: Enable strict type checking.forceConsistentCasingInFileNames: true: Disallow inconsistently-cased references to the same file.
…een ES6/7 and typescript compilation.
…x default export of DashboardList and run prettier on eslintrc
6 tasks
|
Thanks!! |
ghost
mentioned this pull request
andrewdever
pushed a commit
to andrewdever/redash
that referenced
this pull request
Oct 5, 2020
* TASK Add typescript dependencies to package.json * TASK Add typescript to build process and npm scripts and TASK Move example components to typescript and add an example definition file. * TASK Move back to ts-loader instead of babel typescript preset * FIX Remove unnecessary changes * FIX Explicitly mention tsconfig file in webpack.config.js to avoid `error while parsing tsconfig.json, The 'files' list in config file 'tsconfig.json' is empty` See (TypeStrong/ts-loader#405 (comment)) * FIX Move tsconfig to client subdirectory to make it accessible in docker container (only webpack.config.js is copied over from root folder in Dockerfile) * TASK Move from ts-loader to babel to reduce compatibility issues between ES6/7 and typescript compilation. * TASK Add types for classnames, hoist-non-react-statics and lodash. Fix default export of DashboardList and run prettier on eslintrc * Run npm install * Trigger tests * Run npm install 2 * Trigger tests
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this? (check all applicable)
Description
Adds the typescript loader to webpack config and package.json, adds typescript specific linting to eslint and updates npm scripts in package.json to include
.tsand.tsx.I added two examples of using TypeScript:
.d.tscompanion file that adds type definition.Both examples show how we can move existing code over to TypeScript step by step.
Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Dashboard stays the same: