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

[Infra UI] Add and use typed styled-components theme provider #33607

Conversation

weltenwort
Copy link
Member

@weltenwort weltenwort commented Mar 20, 2019

Summary

This adds a specialization of the styled-components api, which is correctly typed according to the new theme variable type definitions included in EUI (elastic/eui#1750) and made available in Kibana via #33873.

Type-checked EUI theme variables make it more likely that breaking changes in the EUI variable definitions are noticed, which would have prevented #33411 and similar issues. This is has been attempted before in #24425 and #25516 using different means.

Since this is only a typing change, it should not have any visible effect except for the fixes for bugs that were discovered due to these changes.

Benefits

The typed styled components API allows the TypeScript language server to provide auto-completion on all available EUI variables:

image

It also enables the TypeScript compiler to check for presence of accesses variables:

image

All the usual refactorings and auto-fixes also work, of course:

image

Using the new typed eui theme

Making use of the types is a simple as replacing the imports from styled-components, e.g.

import styled from 'styled-components';

,with imports from the eui_styled_componenets module located in the x-pack/common directory:

import euiStyled from '../../../../../common/eui_styled_components';

Checklist

For maintainers

@weltenwort weltenwort added [zube]: In Progress v8.0.0 Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.2.0 labels Mar 20, 2019
@weltenwort weltenwort self-assigned this Mar 20, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

This re-exports a variant of the styled-components api, which is
correctly typed according to the new theme variable type definitions
included in EUI.
@weltenwort weltenwort force-pushed the enhancement-add-typed-eui-styled-components branch from 0b59039 to 408eadd Compare March 28, 2019 11:56
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@weltenwort weltenwort marked this pull request as ready for review March 28, 2019 13:48
@skh skh removed request for simianhacker and Kerry350 April 1, 2019 12:54
Copy link
Contributor

@skh skh left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@weltenwort weltenwort changed the title Add and use typed styled-components theme provider [Infra UI] Add and use typed styled-components theme provider Apr 1, 2019
@weltenwort weltenwort merged commit d625136 into elastic:master Apr 1, 2019
weltenwort added a commit to weltenwort/kibana that referenced this pull request Apr 2, 2019
…c#33607)

This adds a specialization of the styled-components api, which is correctly typed according to the new theme variable type definitions included in EUI (elastic/eui#1750) and made available in Kibana via elastic#33873.
weltenwort added a commit that referenced this pull request Apr 2, 2019
…33607) (#34303)

Backports the following commits to 7.x:
 - [Infra UI] Add and use typed styled-components theme provider  (#33607)
@weltenwort weltenwort added the non-issue Indicates to automation that a pull request should not appear in the release notes label May 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
non-issue Indicates to automation that a pull request should not appear in the release notes review Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.2.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants