Fixes #12456 : Migrate 'components/widgets/settings' module and associated tests to TypeScript

[M-ZubairAhmed]

Summary

Migrated following javascript files to Typescript
components/widgets/settings/-

• bool_setting & test
• radio_setting & test
• setting & test
• text_setting & test

Ticket Link

Fixes mattermost/mattermost#12456

[M-ZubairAhmed]

Linting are failing @hanzei this is because we removed prop-types missing in props validation react/prop-types since we are using typescript

[M-ZubairAhmed]

Also let me know if this is a good approach (working with TS first time) so i can continue on other two files

[devinbinnie]

Hey @M-ZubairAhmed! Approach looks good, just a couple small comments.
As for the lint errors, though should be disappearing once you remove PropTypes from the file, so I'm not sure what's happening there.

[M-ZubairAhmed]

Hey @M-ZubairAhmed! Approach looks good, just a couple small comments.
As for the lint errors, though should be disappearing once you remove PropTypes from the file, so I'm not sure what's happening there.

This seems to be happening only functional components not for class components

[M-ZubairAhmed]

@devinbinnie i have fixed the changes

[saturninoabril]

Tested and looks good, with some comments. Also the test failed on npm run check

[devinbinnie]

@M-ZubairAhmed take a look at components/loading_image_preview.tsx, it's a functional component that is passing the linter. Maybe there's something in there that would help?

[devinbinnie]

@M-ZubairAhmed You have some tests failing:

Summary of all failing tests
 FAIL  components/interactive_dialog/dialog_element/dialog_element.test.jsx
  ● components/interactive_dialog/DialogElement › subtype email

    TypeError: Cannot read property 'includes' of undefined

      127 |                 maxLength = maxLength || TEXT_DEFAULT_MAX_LENGTH;
      128 | 
    > 129 |                 if (subtype && TextSetting.validTypes.includes(subtype)) {
          |                                                       ^
      130 |                     type = subtype;
      131 |                 } else {
      132 |                     type = 'input';

      at DialogElement.includes [as render] (components/interactive_dialog/dialog_element/dialog_element.jsx:129:55)
      at ReactShallowRenderer._mountClassComponent (node_modules/react-test-renderer/cjs/react-test-renderer-shallow.development.js:809:37)
      at ReactShallowRenderer.render (node_modules/react-test-renderer/cjs/react-test-renderer-shallow.development.js:738:14)
      at render (node_modules/enzyme-adapter-react-16/src/ReactSixteenAdapter.js:666:53)
      at fn (node_modules/enzyme-adapter-utils/src/Utils.js:99:18)
      at Object.render (node_modules/enzyme-adapter-react-16/src/ReactSixteenAdapter.js:666:18)
      at new render (node_modules/enzyme/src/ShallowWrapper.js:397:22)
      at shallow (node_modules/enzyme/src/shallow.js:10:10)
      at Object.it (components/interactive_dialog/dialog_element/dialog_element.test.jsx:50:25)

  ● components/interactive_dialog/DialogElement › subtype invalid

    TypeError: Cannot read property 'includes' of undefined

      127 |                 maxLength = maxLength || TEXT_DEFAULT_MAX_LENGTH;
      128 | 
    > 129 |                 if (subtype && TextSetting.validTypes.includes(subtype)) {
          |                                                       ^
      130 |                     type = subtype;
      131 |                 } else {
      132 |                     type = 'input';

      at DialogElement.includes [as render] (components/interactive_dialog/dialog_element/dialog_element.jsx:129:55)
      at ReactShallowRenderer._mountClassComponent (node_modules/react-test-renderer/cjs/react-test-renderer-shallow.development.js:809:37)
      at ReactShallowRenderer.render (node_modules/react-test-renderer/cjs/react-test-renderer-shallow.development.js:738:14)
      at render (node_modules/enzyme-adapter-react-16/src/ReactSixteenAdapter.js:666:53)
      at fn (node_modules/enzyme-adapter-utils/src/Utils.js:99:18)
      at Object.render (node_modules/enzyme-adapter-react-16/src/ReactSixteenAdapter.js:666:18)
      at new render (node_modules/enzyme/src/ShallowWrapper.js:397:22)
      at shallow (node_modules/enzyme/src/shallow.js:10:10)
      at Object.it (components/interactive_dialog/dialog_element/dialog_element.test.jsx:65:25)

  ● components/interactive_dialog/DialogElement › subtype password

    TypeError: Cannot read property 'includes' of undefined

      127 |                 maxLength = maxLength || TEXT_DEFAULT_MAX_LENGTH;
      128 | 
    > 129 |                 if (subtype && TextSetting.validTypes.includes(subtype)) {
          |                                                       ^
      130 |                     type = subtype;
      131 |                 } else {
      132 |                     type = 'input';

      at DialogElement.includes [as render] (components/interactive_dialog/dialog_element/dialog_element.jsx:129:55)
      at ReactShallowRenderer._mountClassComponent (node_modules/react-test-renderer/cjs/react-test-renderer-shallow.development.js:809:37)
      at ReactShallowRenderer.render (node_modules/react-test-renderer/cjs/react-test-renderer-shallow.development.js:738:14)
      at render (node_modules/enzyme-adapter-react-16/src/ReactSixteenAdapter.js:666:53)
      at fn (node_modules/enzyme-adapter-utils/src/Utils.js:99:18)
      at Object.render (node_modules/enzyme-adapter-react-16/src/ReactSixteenAdapter.js:666:18)
      at new render (node_modules/enzyme/src/ShallowWrapper.js:397:22)
      at shallow (node_modules/enzyme/src/shallow.js:10:10)
      at Object.it (components/interactive_dialog/dialog_element/dialog_element.test.jsx:80:25)

 FAIL  components/widgets/settings/text_setting.test.tsx
  ● components/widgets/settings/TextSetting › render component with required props

    expect(received).toMatchInlineSnapshot(snapshot)

    Snapshot name: `components/widgets/settings/TextSetting render component with required props 1`

    - Snapshot
    + Received

    @@ -5,11 +5,11 @@
        labelClassName=""
      >
        <input
          className="form-control"
          id="string.id"
    -     maxLength={null}
    +     maxLength={-1}
          onChange={[Function]}
          type="input"
          value="some value"
        />
      </Setting>

      17 |             />
      18 |         );
    > 19 |         expect(wrapper).toMatchInlineSnapshot(`
         |                         ^
      20 | <Setting
      21 |   inputClassName=""
      22 |   inputId="string.id"

      at Object.toMatchInlineSnapshot (components/widgets/settings/text_setting.test.tsx:19:25)

  ● components/widgets/settings/TextSetting › render with textarea type

    expect(received).toMatchInlineSnapshot(snapshot)

    Snapshot name: `components/widgets/settings/TextSetting render with textarea type 1`

    - Snapshot
    + Received

    @@ -5,12 +5,12 @@
        labelClassName=""
      >
        <textarea
          className="form-control"
          id="string.id"
    -     maxLength={null}
    +     maxLength={-1}
          onChange={[Function]}
    -     rows="5"
    +     rows={5}
          style={Object {}}
          value="some value"
        />
      </Setting>

      47 |             />
      48 |         );
    > 49 |         expect(wrapper).toMatchInlineSnapshot(`
         |                         ^
      50 | <Setting
      51 |   inputClassName=""
      52 |   inputId="string.id"

      at Object.toMatchInlineSnapshot (components/widgets/settings/text_setting.test.tsx:49:25)


Snapshot Summary
 › 2 snapshots failed from 1 test suite. Inspect your code changes or run `npm run test-ci -- -u` to update them.

[M-ZubairAhmed]

Working on it

[saturninoabril]

@M-ZubairAhmed Thanks, LGTM

[devinbinnie]

Looks good, thanks @M-ZubairAhmed!

[mattermod]

This issue has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

/cc @jasonblais @hanzei

[M-ZubairAhmed]

How is this stale PR, when it was approved by the team member. It is a COMPLETE pr

[jasonblais]

Hey @M-ZubairAhmed! Looks like it just needs one more review from @catalintomai and it's ready to go.

@catalintomai when you have time, can you help review this PR from @M-ZubairAhmed?

[catalintomai]

Hey @M-ZubairAhmed! Looks like it just needs one more review from @catalintomai and it's ready to go.

@catalintomai when you have time, can you help review this PR from @M-ZubairAhmed?

@jasonblais , how many core committer review approvals do community members PRs need? I believe this one had two already.

[jasonblais]

@catalintomai Each PR requires two dev reviews, and a QA review where applicable. I believe Saturn's review would apply for QA review in this case.

[catalintomai]

@catalintomai Each PR requires two dev reviews, and a QA review where applicable. I believe Saturn's review would apply for QA review in this case.

Thanks. Saturnino's infotag above has "Core Committer" in it. Maybe we should update the doc and explicitly refer to dev core committers as opposed to QA Core Committers/or QA short.

[M-ZubairAhmed]

I am very impressed by review process of this project, let me check out more issues so i can contribute to this project.