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

Upgrading TypeScript to 3.7.2 #3295

Merged
merged 7 commits into from
Apr 15, 2020
Merged

Upgrading TypeScript to 3.7.2 #3295

merged 7 commits into from
Apr 15, 2020

Conversation

anishagg17
Copy link
Contributor

@anishagg17 anishagg17 commented Apr 10, 2020

Summary

Closes #3292

Checklist

- [ ] Check against all themes for compatibility in both light and dark modes
- [ ] Checked in mobile
- [ ] Checked in IE11 and Firefox
- [ ] Props have proper autodocs
- [ ] Added documentation examples
- [ ] Added or updated jest tests

  • Checked for breaking changes and labeled appropriately
    - [ ] Checked for accessibility including keyboard-only and screenreader modes
  • A changelog entry exists and is marked appropriately

@kibanamachine
Copy link

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@anishagg17
Copy link
Contributor Author

Screenshot 2020-04-10 at 3 46 09 PM

this line is not ignored even // @ts-ignore-next-line is used

@chandlerprall chandlerprall self-assigned this Apr 10, 2020
@chandlerprall chandlerprall self-requested a review April 10, 2020 16:01
@anishagg17
Copy link
Contributor Author

Screenshot 2020-04-10 at 9 33 11 PM

This is the second breaking Change caused, rest all have been fixed

@anishagg17 anishagg17 marked this pull request as ready for review April 10, 2020 16:03
@chandlerprall
Copy link
Contributor

data_grid.test.tsx

Needs a second ignore comment,

      // @ts-ignore-next-line
      columnSorter
        .find('EuiSwitch')
        .props()
        // @ts-ignore-next-line
        .onChange();

observer.ts

Looks like it's an issue with changes to @types/react -

        // React.Props<T> is now deprecated, which means that the `children`
        // property is not available on `P` by default, even though you can
        // always pass children as variadic arguments to `createElement`.
        // In the future, if we can define its call signature conditionally
        // on the existence of `children` in `P`, then we should remove this.
        readonly props: Readonly<P> & Readonly<{ children?: ReactNode }>;

but can be fixed in the EUI component with:

    const props: BaseProps = this.props;
    return props.children(this.updateChildNode);

package.json

devDependencies can both be updated too

    "@types/react": "^16.9.34",
    "@types/react-dom": "^16.9.6",

yarn.lock

Changing the above dependencies will create an issue with yarn which needs a manual resolution:

  • find the "@types/react@*": and "@types/react@^16.9.23": entries in yarn.lock and delete them
  • re-run yarn
  • commit the resulting yarn.lock

CHANGELOG.md

Needs a breaking change entry

@chandlerprall
Copy link
Contributor

Merged master into this branch to get the changelog entry into the right location, as we released an EUI version yesterday. I also removed the react & react-dom mentions in the changelog as those changes only affect the devDependencies and not any published version.

jenkins test this

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_3295/

@chandlerprall
Copy link
Contributor

Jest OOMed. jenkins test this

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_3295/

@chandlerprall chandlerprall merged commit dd296ce into elastic:master Apr 15, 2020
@anishagg17 anishagg17 deleted the ty branch April 15, 2020 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade Typescript in EUI to 3.7.2
5 participants