Skip to content

Cleanup scales export for better import strategy#5953

Merged
simonbrunel merged 7 commits intochartjs:masterfrom
simonbrunel:chore/importable-scales
Jan 5, 2019
Merged

Cleanup scales export for better import strategy#5953
simonbrunel merged 7 commits intochartjs:masterfrom
simonbrunel:chore/importable-scales

Conversation

@simonbrunel
Copy link
Member

Scales now export a function responsible to register the scale class and associated defaults while the definition has been moved outside the exported function.

Relates #4478
Closes #5941

Scales now export a function responsible to register the scale class and associated defaults while the definition has been moved outside the exported function.
@simonbrunel simonbrunel added this to the Version 2.8 milestone Jan 4, 2019
@simonbrunel simonbrunel force-pushed the chore/importable-scales branch from 1d00765 to 93850b6 Compare January 4, 2019 17:30
Copy link
Member

@kurkle kurkle left a comment

Choose a reason for hiding this comment

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

Cant fully review on mobile, whitespace ignoring setting is not obeyed on larger diffs. Will do full review later.

benmccann
benmccann previously approved these changes Jan 4, 2019
src/chart.js Outdated
Chart.elements = require('./elements/index');
Chart.Interaction = require('./core/core.interaction');
Chart.layouts = require('./core/core.layouts');
Chart.LinearScaleBase = require('./scales/scale.linearbase');
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a comment to remove this at v3 since it should be internal only?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

@etimberg etimberg left a comment

Choose a reason for hiding this comment

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

Just one minor comment. Looks good

Copy link
Member

@kurkle kurkle left a comment

Choose a reason for hiding this comment

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

Had to look hard, but found eventually a line to comment on 😈

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.

4 participants