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

Use lodash-es and upgrade to TS@4.4 #3589

Merged
merged 8 commits into from
Nov 17, 2021
Merged

Use lodash-es and upgrade to TS@4.4 #3589

merged 8 commits into from
Nov 17, 2021

Conversation

michael-yx-wu
Copy link
Contributor

No description provided.

@michael-yx-wu michael-yx-wu marked this pull request as ready for review November 16, 2021 23:21
@@ -56,7 +56,7 @@ export function makeSymbolCanvasDrawStep(

// check attributes and symbol type
const attrsSame = isAttributeValuesEqual(prevAttrs, attrs, ContextStyleAttrs);
const symbolGenerator = symbolAccessor(datum, index, this._dataset);
const symbolGenerator = symbolAccessor(datum, index, dataset);
Copy link
Contributor Author

@michael-yx-wu michael-yx-wu Nov 17, 2021

Choose a reason for hiding this comment

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

I wonder why this ever worked, since the this in a browser context refers to the window object and doesn't make any sense here. Maybe a transpilation bug that is surfacing now that we've upgraded typescript?

@blueprint-bot
Copy link

Remove weird reference to this

Demo: quicktests | fiddle

@michael-yx-wu
Copy link
Contributor Author

michael-yx-wu commented Nov 17, 2021

A lot of casting to SVGRect, DOMRect, ClientRect, etc. because newer definitions for these require us to define things like the x, y, and toJSON properties. Given things were working fine before, I think it's okay to just forcibly cast

src/memoize/memoizeProjectors.ts Outdated Show resolved Hide resolved
src/plots/rectanglePlot.ts Outdated Show resolved Hide resolved
src/plots/scatterPlot.ts Outdated Show resolved Hide resolved
src/utils/domUtils.ts Outdated Show resolved Hide resolved
src/utils/stackingUtils.ts Outdated Show resolved Hide resolved
@blueprint-bot
Copy link

Pick instead of casting and import type

Demo: quicktests | fiddle

yarn.lock Outdated Show resolved Hide resolved
@blueprint-bot
Copy link

Fix indentation

Demo: quicktests | fiddle

@blueprint-bot
Copy link

Remove unnecessary casting

Demo: quicktests | fiddle

Comment on lines +4480 to +4481
lodash-es@^4.17.15:
version "4.17.15"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Matches our internal version

@blueprint-bot
Copy link

Use lodash-es@4.7.15

Demo: quicktests | fiddle

@michael-yx-wu michael-yx-wu merged commit 2ae4d07 into develop Nov 17, 2021
@michael-yx-wu michael-yx-wu deleted the mw/lodash-es branch November 17, 2021 21:50
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

4 participants