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

Convert canvas-extract to TypeScript #6503

Merged
merged 7 commits into from
May 4, 2020
Merged

Conversation

Zyie
Copy link
Member

@Zyie Zyie commented Mar 27, 2020

Description of change

Nice and simple!

@bigtimebuddy
Copy link
Member

I actually think your instincts were right about the GlobalMixins, however, I think we need them to define CanvasRender plugins and Renderer plugins.

Currently, this is defined pretty loosely here: https://github.com/pixijs/pixi.js/blob/dev/packages/core/src/AbstractRenderer.ts#L36-L39

That means users don't get typings for doing things like renderer.plugins.extract.canvas().

We should do this as a follow-up fix for all the plugins.

@ivanpopelyshev
Copy link
Collaborator

plugins can be moved to GlobalMixins in separate PR , this PR is fine as it is. circular type deps? we have them anyway, we can fix stuff a bit later.

@bigtimebuddy bigtimebuddy added this to the v5.3.0 milestone Mar 28, 2020
@bigtimebuddy
Copy link
Member

@Zyie can remove CanvasExtract from the CanvasRenderer? Otherwise this is good to go

@bigtimebuddy bigtimebuddy added the ✅ Ready To Merge Helpful when issues are in the queue waiting to get merged. This means the PR is completed and has t label May 1, 2020
@codecov-io
Copy link

codecov-io commented May 1, 2020

Codecov Report

Merging #6503 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev    #6503   +/-   ##
=======================================
  Coverage   82.34%   82.34%           
=======================================
  Files          38       38           
  Lines        1903     1903           
=======================================
  Hits         1567     1567           
  Misses        336      336           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1fa945e...4872f0c. Read the comment docs.

@bigtimebuddy
Copy link
Member

I've deprecated CanvasRenderer#extract as part of this PR. This is an anti-pattern with the rest of the Renderer plugins, which hang off the plugins object.

@bigtimebuddy bigtimebuddy merged commit 0a62c03 into dev May 4, 2020
@bigtimebuddy bigtimebuddy deleted the dev-canvas-extract-typescript branch May 4, 2020 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✅ Ready To Merge Helpful when issues are in the queue waiting to get merged. This means the PR is completed and has t
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants