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

ECharts: support tree shaking #2001

Closed
wants to merge 1 commit into from

Conversation

vhdirk
Copy link
Contributor

@vhdirk vhdirk commented Dec 9, 2022

Description

As mentioned in #1933, the user currently has make the echarts library globally available. This PR provides a way to resolve that and enable full tree-shaking support. This should also be a partial fix for #1775.

What's included?

This PR introduces a new TD_ECHARTS_CONFIG injectiontoken that is used to pass the echarts library reference to CovalentBaseEchartsModule.

import * as echartsLibraryReference from 'echarts';

...
  CovalentBaseEchartsModule.forRoot({
    echarts: echartsLibraryReference,
    defaultThemes: false
  }),

or  

providers: [{
  provide: TD_ECHARTS_CONFIG,
  useValue: {
    echarts: echartsLibraryReference,
    defaultThemes: false
  }
}]

Test Steps

  • npm run start
  • then this
  • finally this

General Tests for Every PR

  • npm run start still works.
  • npm run lint passes.
  • npm run stylelint passes.
  • npm test passes and code coverage is not lower.
  • npm run build still works.
Screenshots or link to StackBlitz/Plunker

Copy link
Collaborator

@owilliams320 owilliams320 left a comment

Choose a reason for hiding this comment

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

@vhdirk Can you seperate out the schematic and a11y additions if that is not related to echarts?

@owilliams320 owilliams320 changed the base branch from main to beta January 19, 2023 22:21
@owilliams320 owilliams320 deleted the branch Teradata:beta March 10, 2023 18:20
@owilliams320 owilliams320 reopened this Mar 10, 2023
@owilliams320 owilliams320 changed the base branch from beta to main March 10, 2023 18:23
@owilliams320 owilliams320 changed the base branch from main to beta March 10, 2023 18:23
@owilliams320
Copy link
Collaborator

@vhdirk looks like the tests and e2e job is failing can you fix this?

@vhdirk
Copy link
Contributor Author

vhdirk commented Mar 16, 2023

@owilliams320 unfortunately, I'm having some time of for personal reasons. I won't be able to work on this for a while.

1 similar comment
@vhdirk
Copy link
Contributor Author

vhdirk commented Mar 16, 2023

@owilliams320 unfortunately, I'm having some time of for personal reasons. I won't be able to work on this for a while.

@owilliams320
Copy link
Collaborator

owilliams320 commented Mar 17, 2023

Sure no problem. We will close this and come back to it when you have time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants