Add typed-css-modules-webpack-plugin to support type checking #3192

onursumer · 2020-05-04T21:52:09Z

Configure typed-css-mdoules-webpack-plugin to auto generate typings for every scss module (files matching the pattern *.module.scss)
For each scss module file, it generates a TS type definition file in the form of *.module.scss.d.ts
Works for both build and watch modes.

Checks

Has tests or has a separate issue that describes the types of test that should be created. If no test is included it should explicitly be mentioned in the PR why there is no test.
The commit log is comprehensible. It follows 7 rules of great commit messages. For most PRs a single commit should suffice, in some cases multiple topical commits can be useful. During review it is ok to see tiny commits (e.g. Fix reviewer comments), but right before the code gets merged to master or rc branch, any such commits should be squashed since they are useless to the other developers. Definitely avoid merge commits, use rebase instead.
Is this PR adding logic based on one or more clinical attributes? If yes, please make sure validation for this attribute is also present in the data validation / data loading layers (in backend repo) and documented in File-Formats Clinical data section!

onursumer · 2020-05-06T17:53:34Z

src/shared/components/copyDownloadControls/copyDownloadControls.module.scss

@@ -1,5 +1,5 @@
 .centered-modal-dialog {
-    // TODO centering doesn't work on the patient view, disabling for now...
+    // TODO centering does not work on the patient view, disabling for now...


parser doesn't like the ' character

onursumer · 2020-05-06T17:54:01Z

src/shared/components/errorBox/ErrorBox.tsx

 import classNames from 'classnames';

-const styles = styles_any as {


we don't need to manually add styles anymore

onursumer · 2020-05-06T17:55:35Z

webpack.config.js

+        new TypedCssModulesPlugin({
+            globPattern: 'src/**/*.module.scss',
+        }),


this does the magic

onursumer · 2020-05-06T17:56:35Z

src/pages/patientView/patientHeader/styles.module.scss.d.ts

@@ -0,0 +1,3 @@
+


The corresponding styling file is empty

onursumer · 2020-05-06T17:57:27Z

src/pages/studyView/table/FixedHeaderTable.tsx

@@ -335,7 +335,9 @@ export default class FixedHeaderTable<T> extends React.Component<
                            {selectionOperation}{' '}
                            <i
                                className={
-                                    styles[selectionOperation.toLowerCase()]
+                                    (styles as any)[


Need to do as any when referencing with a variable string

onursumer · 2020-05-06T17:58:44Z

src/shared/components/query/styles/styles.module.scss

-        //> div:nth-child(2) {
-        //  margin-top:0px !important;
-        //}
+        /*> div:nth-child(2) {


Parser doesn't like line comments

adamabeshouse · 2020-05-06T18:48:59Z

src/pages/groupComparison/styles.module.scss.d.ts

+  readonly "noGroupsMessage": string;
+  readonly "survivalPlotHeader": string;
+  readonly "survivalPlotHeaderContainer": string;
+  readonly "survivalTypeOptions": string;


is this auto created or do we have to go in and add this every time we add a class?

auto created, and works fine in the watch mode.

adamabeshouse · 2020-05-06T18:49:27Z

src/pages/patientView/clinicalInformation/style/local.module.scss.d.ts

@@ -0,0 +1,3 @@
+


empty file?

yep, we have an empty style file :)

Sorry, the empty one was the other file. Good catch!

Here the source scss doesn't really have any named references, it shouldn't be defined as a module but probably a regular scss.

seems like we don't even import local.module.scss anywhere, probably safe to remove it completely.

adamabeshouse · 2020-05-06T18:49:49Z

src/pages/patientView/patientHeader/PatientHeader.tsx

@@ -10,7 +10,8 @@ import {
 } from 'cbioportal-frontend-commons';
 import SampleManager from '../SampleManager';

-import styles from './styles.module.scss';
+// TODO styles.module.scss is empty!
+// import styles from './styles.module.scss';


remove comments?

yeah, eventually we should either remove those invalid usage or update them

adamabeshouse · 2020-05-06T18:50:13Z

src/shared/components/GeneSelectionBox/OQLTextArea.tsx

@@ -194,7 +194,8 @@ export default class OQLTextArea extends React.Component<
                break;
        }
        this.geneQuery
-            ? classNames.push(styles.notEmpty)
+            ? // TODO no such style called `notEmpty`
+              classNames.push(/* styles.notEmpty */ '')


get rid of this altogether then?

most likely we will end up removing all the invalid references before merging the PR, but keeping them for now just in case someone wants to comment.

adamabeshouse · 2020-05-06T18:51:15Z

src/shared/components/annotation/genomeNexus/Sift.tsx

@@ -82,7 +84,7 @@ export default class Sift extends React.Component<ISiftProps, {}> {
                        <td>
                            <span
                                className={
-                                    tooltipStyles[
+                                    (tooltipStyles as any)[
                                        `sift-${this.props.siftPrediction}`


instead you could try tooltipStyles[`sift-$....` as keyof tooltipStyles`]. Not sure if it would work but it might

hmm, I'll give it a try

this doesn't seem to work, leaving it as any for now

adamabeshouse · 2020-05-06T18:51:27Z

src/shared/components/annotation/genomeNexus/Sift.tsx

@@ -43,7 +43,9 @@ export default class Sift extends React.Component<ISiftProps, {}> {
                <span
                    className={classNames(
                        annotationStyles['annotation-item-text'],
-                        tooltipStyles[`sift-${this.props.siftPrediction}`]
+                        (tooltipStyles as any)[
+                            `sift-${this.props.siftPrediction}`


same thing here as below vvv

onursumer · 2020-05-07T00:04:58Z

.prettierignore

@@ -16,6 +16,7 @@
 *pegjs
 *.template
 *.ejs
+*module.scss.d.ts


ignore auto generated module typing files

…s modules Signed-off-by: Onur Sumer <s.onur.sumer@gmail.com>

onursumer requested a review from alisman May 4, 2020 21:52

onursumer force-pushed the typed-css-modules branch 2 times, most recently from cd06422 to 478b243 Compare May 6, 2020 02:58

onursumer commented May 6, 2020

View reviewed changes

onursumer marked this pull request as ready for review May 6, 2020 17:59

onursumer requested review from inodb, zhx828 and adamabeshouse May 6, 2020 18:00

onursumer added the DO NOT MERGE label May 6, 2020

adamabeshouse reviewed May 6, 2020

View reviewed changes

onursumer force-pushed the typed-css-modules branch 2 times, most recently from 544a6d4 to bbda3c8 Compare May 6, 2020 22:04

onursumer removed the DO NOT MERGE label May 6, 2020

onursumer force-pushed the typed-css-modules branch from bbda3c8 to bf3c38f Compare May 6, 2020 23:50

onursumer commented May 7, 2020

View reviewed changes

.prettierignore

@@ -16,6 +16,7 @@

*pegjs

*.template

*.ejs

*module.scss.d.ts

Copy link

Member Author

onursumer May 7, 2020

Choose a reason for hiding this comment

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

ignore auto generated module typing files

adamabeshouse approved these changes May 7, 2020

View reviewed changes

onursumer force-pushed the typed-css-modules branch 5 times, most recently from dd91731 to 9eb7572 Compare May 7, 2020 22:25

add typed-css-modules-webpack-plugin to support type checking for scs…

de8f1e8

…s modules Signed-off-by: Onur Sumer <s.onur.sumer@gmail.com>

onursumer force-pushed the typed-css-modules branch from 9eb7572 to de8f1e8 Compare May 8, 2020 02:24

alisman approved these changes May 8, 2020

View reviewed changes

alisman merged commit 2208647 into cBioPortal:master May 8, 2020

alisman deleted the typed-css-modules branch May 8, 2020 14:29

inodb added the chore This PR is related to code maintenance label May 14, 2020

This was referenced Jun 1, 2020

Split OncoKbCard to small components && Add e2e tests for OncoKB #3248

Merged

Add css type checking for modules under packages cBioPortal/cbioportal#7559

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add typed-css-modules-webpack-plugin to support type checking #3192

Add typed-css-modules-webpack-plugin to support type checking #3192

onursumer commented May 4, 2020 •

edited

onursumer May 6, 2020

onursumer May 6, 2020

onursumer May 6, 2020

onursumer May 6, 2020

onursumer May 6, 2020

onursumer May 6, 2020

adamabeshouse May 6, 2020

onursumer May 6, 2020

adamabeshouse May 6, 2020

onursumer May 6, 2020

onursumer May 6, 2020

onursumer May 6, 2020

adamabeshouse May 6, 2020

onursumer May 6, 2020

adamabeshouse May 6, 2020

onursumer May 6, 2020

adamabeshouse May 6, 2020

onursumer May 6, 2020

onursumer May 6, 2020

adamabeshouse May 6, 2020

onursumer May 7, 2020

		import classNames from 'classnames';

		const styles = styles_any as {

Add typed-css-modules-webpack-plugin to support type checking #3192

Add typed-css-modules-webpack-plugin to support type checking #3192

Conversation

onursumer commented May 4, 2020 • edited

Checks

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

onursumer commented May 4, 2020 •

edited