Skip to content

Update dependencies and alter privacy of functions#7138

Merged
etimberg merged 6 commits intochartjs:masterfrom
kurkle:deps
Feb 22, 2020
Merged

Update dependencies and alter privacy of functions#7138
etimberg merged 6 commits intochartjs:masterfrom
kurkle:deps

Conversation

@kurkle
Copy link
Member

@kurkle kurkle commented Feb 21, 2020

So, this was supposed to be a simple typescript update. But as I found out, in typescript 3.8 the @private means private to the class (and not to the lib as its been used here)

Some of those I changed to @protected (accessible from extended classes)
Some I made public (and removed the underscore).

@kurkle kurkle changed the title Update dependencies Update dependencies and alter privacy of functions Feb 21, 2020
Copy link
Contributor

@benmccann benmccann left a comment

Choose a reason for hiding this comment

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

I think I would remove the underscore from the @protected methods. The underscore basically indicates that they're internal methods, but if it's required for people to override them in order to implement a scale, etc. then they really can't just be internal

* @private
*/
_getLabelCapacity(exampleTime) {
getLabelCapacity(exampleTime) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is only used in this file. Can we keep it private?

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, using @ts-ignore

@kurkle
Copy link
Member Author

kurkle commented Feb 21, 2020

Removed the underscores from protected. tsc does not know meta.controller is a DatasetController etc, so there might be quite a few things still "not right"

Copy link
Contributor

@benmccann benmccann left a comment

Choose a reason for hiding this comment

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

thanks. this was a big change

* @type {string[]|object}
* @private
*/
DatasetController.prototype._dataElementOptions = [
Copy link
Contributor

Choose a reason for hiding this comment

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

We should make this @protected and remove the underscore? (same with _datasetElementOptions)

Copy link
Member Author

Choose a reason for hiding this comment

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

ts does not like them protected (not in class). Could probably do that in #7125
Removed the underscore though.

// all borders are drawn for floating bar
/* TODO: float bars border skipping magic
if (me._getParsed(i)._custom) {
if (me.getParsed(i)._custom) {
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 just remove this commented code?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe let's handle separately? It may be helpful to leave this until we fix the TODO and fixing the TODO seems big enough that it should be a separate item. I put a reminder in the v3 tracking bug to go through and fix any TODOs so that we don't forget

@etimberg etimberg merged commit d801e56 into chartjs:master Feb 22, 2020
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.

3 participants