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

[SIP-9] Introduce TypeScript #6120

Merged
merged 4 commits into from
Oct 24, 2018
Merged

Conversation

xtinec
Copy link
Contributor

@xtinec xtinec commented Oct 16, 2018

  • Introduce TypeScript and co to both source and tests
  • Define alias for src directory in both webpack config and jest config so we can avoid using long relative paths like ../../src in both source and tests
  • Type check feature flags system to prevent typos of flag names
  • Change the feature flags system and the flags on window instead of populating them through the state tree. When introducing the first SCOPED_FILTER feature flag, it became too difficult to pipe the flags through the state initializers and layers of components and containers (the resulting code is hard to read and has a handful of methods taking an additional feature flag map parameter). Given that feature flags don't change throughout the life time of the app, it is better to leave them on window for easy access than piping them through the global state tree, which is meant to store the state of the app which changes frequently.
  • Add a barebone filter panel that only shows when the SCOPED_FILTER feature flag is on

image

@mistercrunch @betodealmeida @williaster @kristw @john-bodley @ttannis

@xtinec xtinec force-pushed the typescript-setup branch 3 times, most recently from f1bed58 to 1cebfc9 Compare October 17, 2018 22:18
@@ -102,8 +109,12 @@ const config = {
},
},
resolve: {
extensions: ['.js', '.jsx'],
alias: {
'@': path.resolve(APP_DIR, './src'),
Copy link
Contributor

Choose a reason for hiding this comment

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

wanted to gauge thoughts on making this more readable like @src or @root, etc. instead of just @.

I don't care that strongly, but do think more readability is generally good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree. More readability is always good. :)
Any preference for @src VS @root? @williaster @kristw @mistercrunch I think I like them equally. hehe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okey-dokey. Renaming it to @src shortly. :)

Copy link
Contributor

Choose a reason for hiding this comment

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

sgtm, don't have a strong pref / like them equally, too 😃

- Introduce TypeScript and co to both source and tests
- Define alias for src directory in both webpack config and jest config so we can avoid using long relative paths like ../../src in both source and tests
- Type check feature flags system to prevent typos of flag names
- Change the feature flags system and the flags on window instead of populating them through the state tree. When introducing the first SCOPED_FILTER feature flag, it became too difficult to pipe the flags through the state initializers and layers of components and containers (the resulting code is hard to read and has a handful of methods taking an additional feature flag map parameter). Given that feature flags don't change throughout the life time of the app, it is better to leave them on window for easy access than piping them through the global state tree, which is meant to store the state of the app which changes frequently.
- Add a barebone filter panel that only shows when the SCOPED_FILTER feature flag is on
- Fixing linting for Javascript files importing Typscript files
- Also fix linting for Javascript files that now leverage the webpack alias for the src directory

- up Typescript and type def versions
Copy link
Contributor

@williaster williaster left a comment

Choose a reason for hiding this comment

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

thanks so much for kicking this off! 🙌 💯

@xtinec
Copy link
Contributor Author

xtinec commented Oct 24, 2018

My pleasure! 🙌

Btw, wound up renaming the webpack alias for the src directory from @ to src (turns out eslint dislikes any alias prefixed with @ unless it is @ itself).

@codecov-io
Copy link

Codecov Report

Merging #6120 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #6120   +/-   ##
=======================================
  Coverage   76.91%   76.91%           
=======================================
  Files          47       47           
  Lines        9362     9362           
=======================================
  Hits         7201     7201           
  Misses       2161     2161

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 9e6b171...ed67d6d. Read the comment docs.

@mistercrunch
Copy link
Member

TypeScript FTW!

@mistercrunch mistercrunch merged commit 5f1eaa4 into apache:master Oct 24, 2018
bipinsoniguavus pushed a commit to ThalesGroup/incubator-superset that referenced this pull request Dec 26, 2018
* [SIP-9] Introduce TypeScript

- Introduce TypeScript and co to both source and tests
- Define alias for src directory in both webpack config and jest config so we can avoid using long relative paths like ../../src in both source and tests
- Type check feature flags system to prevent typos of flag names
- Change the feature flags system and the flags on window instead of populating them through the state tree. When introducing the first SCOPED_FILTER feature flag, it became too difficult to pipe the flags through the state initializers and layers of components and containers (the resulting code is hard to read and has a handful of methods taking an additional feature flag map parameter). Given that feature flags don't change throughout the life time of the app, it is better to leave them on window for easy access than piping them through the global state tree, which is meant to store the state of the app which changes frequently.
- Add a barebone filter panel that only shows when the SCOPED_FILTER feature flag is on

* Remove unnecessary dev-dependency on gl

* - Adding linting for TypeScript files via tslint.
- Fixing linting for Javascript files importing Typscript files
- Also fix linting for Javascript files that now leverage the webpack alias for the src directory

- up Typescript and type def versions

* Rename src directory's webpack alias from @ to src to be more explicit.
@etr2460 etr2460 mentioned this pull request Jan 31, 2020
12 tasks
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.34.0 labels Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.34.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants