Remove defining the DefaultTheme from library DTS

[soGit]

Issue #:

After PR https://github.com/aws/amazon-chime-sdk-component-library-react/pull/589, DefaultTheme and 'styled-components' module declaration appear in the library files. This pull requests allows defining DefaultTheme type on the application level without forcing to library structure.

Ex. Current implementation doesn't allow extending theme colors on the application level.

declare module 'styled-components' {
  export interface DefaultTheme {
    colors: {
      myCustomColorForAnotherLibrary: string;
// TS Error: Subsequent property declarations must have the same type.  Property 'colors' must be of type '{ primary: ColorType; secondary: ColorType; error: ColorType; success: ColorType; info: ColorType; warning: 
    }
  }
}

Testing

1. Have you successfully run npm run build:release locally? Yes

2. How did you test these changes? Tested by checking running 'npm build' locally and checking 'lib' folder.

3. If you made changes to the component library, have you provided corresponding documentation changes? NA

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[ltrung]

Thanks for fixing but could you add more details in the description on what you try to do and what error do you get. I tried to create a new theme here in our react meeting demo and it compiles fine: https://github.com/aws-samples/amazon-chime-sdk/commit/0b3070800258f690a01fc75a5e3d1a9c583b31fb

[soGit]

Thanks for fixing but could you add more details in the description on what you try to do and what error do you get. I tried to create a new theme here in our react meeting demo and it compiles fine: aws-samples/amazon-chime-sdk@0b30708

To reproduce the problem, you need not only create a custom theme, but also use it. Just add myCustomColorForAnotherLibrary color to any component's styles.

color: ${({ theme }) => theme.colors.myCustomColorForAnotherLibrary};

or define the theme type directly

import { DefaultTheme } from 'styled-components';

export const defaultTheme: DefaultTheme = {
  ...lightTheme,
  colors: {
    myCustomColorForAnotherLibrary: '#88b2ff',
  },
};

[ltrung]

DefaultTheme

I tried to use your code snippet but got this error Property 'colors' does not exist on type 'DefaultTheme' . Isn't the DefaultTheme imported directly from style-components empty? I also discussed with another team member and we think you can just create your own theme and use it too.

[soGit]

Here you can find the branch where I've reproduces TS lint error using your project https://github.com/soGit/amazon-chime-sdk/commit/5dbac88fa06750de46fa26c4d40b738ca89aae32

Please make sure that you don't have symbolic link to the amazon-chime-sdk-component-library-react inside node modules

[xuesichao]

Here you can find the branch where I've reproduces TS lint error using your project soGit/amazon-chime-sdk@5dbac88

Please make sure that you don't have symbolic link to the amazon-chime-sdk-component-library-react inside node modules

Hi @soGit, I'm following up on this, could you please provide the detailed steps to reproduce the issue in https://github.com/soGit/amazon-chime-sdk/commit/5dbac88fa06750de46fa26c4d40b738ca89aae32. I don't understand it clearly. I will also try to play with it myself.

[soGit]

Here you can find the branch where I've reproduces TS lint error using your project soGit/amazon-chime-sdk@5dbac88 Please make sure that you don't have symbolic link to the amazon-chime-sdk-component-library-react inside node modules

Hi @soGit, I'm following up on this, could you please provide the detailed steps to reproduce the issue in soGit/amazon-chime-sdk@5dbac88. I don't understand it clearly. I will also try to play with it myself.

1. Clone the project
2. Run in a console npm install
3. Run in a console npm run start:client

As a result, error should be in the console

[xuesichao]

Hi @soGit , thanks for providing the repro steps. I can reproduce the issue with the commit you provided.

This is where we add new color to the theme:

export const defaultTheme = {
  ...lightTheme,
  colors: {
    ... lightTheme.colors,
    myCustomColorForAnotherLibrary: '#88b2ff', <==========
  },
  chat: chatLightTheme,
};

And this is how you consume it, I got the same error as yours in this way:

import styled from "styled-components";

export const StyledP = styled.p`
  padding: 1rem 1rem 1rem 0;
  color: ${({ theme }) => `${theme.colors.myCustomColorForAnotherLibrary}`};
`;

But if I consume it in this way:

1. Using styled component

import styled from "styled-components";
import { defaultTheme } from "../../theme/demoTheme";

export const StyledP = styled.p`
  padding: 1rem 1rem 1rem 0;
  color: ${defaultTheme.colors.myCustomColorForAnotherLibrary}
`;

And use StyledP in the component:

import { StyledP } from './Styled';

...

return (
  props.deviceLabels === DeviceLabels.None ? null :
    <>
      <ControlBarButton icon={icon} onClick={handleClick} label={label} />
      <DevicePermissionPrompt />
      <StyledP>Test</StyledP> <==== Use it here
    </>
);

2. Or use the colors from theme directly:

import { defaultTheme } from '../theme/demoTheme';

...
const color = defaultTheme.colors.myCustomColorForAnotherLibrary;
...
  return (
    <Roster className="roster">
      <RosterHeader
        searchValue={filter}
        onSearch={handleSearch}
        onClose={closeRoster}
        title="Present"
        badge={attendees.length}
      />
      <RosterGroup>{attendeeItems}</RosterGroup>
      <p style={{ color: color }}> // <==== Use it directly here
        Test
      </p>
    </Roster>
  );

Either way it works well. There were no errors and the colors took effect. I pushed the change to this branch: https://github.com/xuesichao/amazon-chime-sdk/tree/reporduce-theme-tslint-error. And here's the result:

Question

So is there any specific reason that you have to consume it in the way you proposed?

import styled from "styled-components";

export const StyledP = styled.p`
  padding: 1rem 1rem 1rem 0;
  color: ${({ theme }) => `${theme.colors.myCustomColorForAnotherLibrary}`};
`;

[soGit]

Hi @soGit , thanks for providing the repro steps. I can reproduce the issue with the commit you provided.

This is where we add new color to the theme:

export const defaultTheme = {
  ...lightTheme,
  colors: {
    ... lightTheme.colors,
    myCustomColorForAnotherLibrary: '#88b2ff', <==========
  },
  chat: chatLightTheme,
};

And this is how you consume it, I got the same error as yours in this way:

import styled from "styled-components";

export const StyledP = styled.p`
  padding: 1rem 1rem 1rem 0;
  color: ${({ theme }) => `${theme.colors.myCustomColorForAnotherLibrary}`};
`;

But if I consume it in this way:

1. Using styled component

import styled from "styled-components";
import { defaultTheme } from "../../theme/demoTheme";

export const StyledP = styled.p`
  padding: 1rem 1rem 1rem 0;
  color: ${defaultTheme.colors.myCustomColorForAnotherLibrary}
`;

And use StyledP in the component:

import { StyledP } from './Styled';

...

return (
  props.deviceLabels === DeviceLabels.None ? null :
    <>
      <ControlBarButton icon={icon} onClick={handleClick} label={label} />
      <DevicePermissionPrompt />
      <StyledP>Test</StyledP> <==== Use it here
    </>
);

2. Or use the colors from theme directly:

import { defaultTheme } from '../theme/demoTheme';

...
const color = defaultTheme.colors.myCustomColorForAnotherLibrary;
...
  return (
    <Roster className="roster">
      <RosterHeader
        searchValue={filter}
        onSearch={handleSearch}
        onClose={closeRoster}
        title="Present"
        badge={attendees.length}
      />
      <RosterGroup>{attendeeItems}</RosterGroup>
      <p style={{ color: color }}> // <==== Use it directly here
        Test
      </p>
    </Roster>
  );

Either way it works well. There were no errors and the colors took effect. I pushed the change to this branch: https://github.com/xuesichao/amazon-chime-sdk/tree/reporduce-theme-tslint-error. And here's the result:

Question

So is there any specific reason that you have to consume it in the way you proposed?

import styled from "styled-components";

export const StyledP = styled.p`
  padding: 1rem 1rem 1rem 0;
  color: ${({ theme }) => `${theme.colors.myCustomColorForAnotherLibrary}`};
`;

Thank you for response. Unfortunately, in the application we have dynamic themes, and don't want to introduce an additional logic to use a theme directly without ThemeProvider. Beside that we have a different UI library that uses dynamic themes as well. You can read more here about why it's important to use theme property and don't use it directly.

I have one question. What are the benefits of a 'styled-components' module declaration in the package build (DTS)files?

[xuesichao]

@soGit , thanks for your information. The PR looks good to us! Thanks for your contribution! I pushed another commit on top of yours to add a log in CHANGELOG.

[xuesichao]

Hi @soGit , this change will be included in the next release, and we are targeting a release this week.

[soGit]

@ltrung, @xuesichao, great news, thanks for your assistance!

[xuesichao]

Hi @soGit we just released v3.3.0. Let us know if this release fixes your issue. Thanks for your patience and using Amazon Chime SDK!