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

Update to PIXI v6 with individual package imports #152

Merged
merged 11 commits into from
Jun 21, 2021

Conversation

qtiki
Copy link
Contributor

@qtiki qtiki commented Jun 8, 2021

Conversion work for #151

Here's a list of what I did to make the package work with PIXI v6 using the individual modules:

  • Had to update dts-bundle-generator dependency because PIXI's type only import type imports did not work with the old one.
  • Had to leave pixi.js as a dev dependency since it's needed for the examples to work.
  • Had to enable esModuleInterop tsc flag for @pixi/utils EventEmitter to work.
  • Updated the buildAmbientTypes.js script to work with multiple imports and verified that the new output is similar to the old one.
  • Tested that all examples under docs work and correctly report PIXI v6 in the debug console.
  • Also specifically tested that all the different containers in the performance test work too.
  • Got rid of a bunch of as any cast workarounds in the LinkedListContainer thanks to improved PIXI types.
  • Didn't touch the tests at all since in package.json the test script is a simple echo done? Maybe they're not implemented yet?

As for some breaking changes I dropped support for v4 for the shared Ticker and Texture.fromImage usage. Since v4 doesn't have the individual packages I'm not sure if it's possible to keep supporting v4 and v6 at the same time?

I didn't yet do anything with the LinkedListContainer renderWebGL, renderAdvancedWebGL and renderCanvas functions. I think they're only for v4 so they could probably be removed if the v4 support is dropped?

I also didn't touch README.md since I'm not sure what is going to happen with v4 support yet.

Feedback would be welcome.

@andrewstart
Copy link
Collaborator

Dropping support for v4 was expected - honestly I was expecting to drop support for v5, but if it still works for that then great!
You are correct that in LinkedListContainer, renderWebGL and renderAdvancedWebGL can be dropped as v4-only methods. renderCanvas is worth keeping as it is used with the canvas rendering plugin in v5/6.

What is here looks good, I think the only further thing to do is to update the manual tests in the /tests folder - replacing the v4 tests with v6 and changing the modules tests to use the individual modules instead of the bundle.

@qtiki
Copy link
Contributor Author

qtiki commented Jun 9, 2021

So I played around with the tests a bit which was quite interesting. Here's a summary of what I did with them in addition to removing the old v4 tests:

pixi-v5-iife

The v5 iife version worked pretty much out of the box just by enabling the esModuleInterop tsc setting.

image

pixi-v5-module

For the v5 module test I had to add one as any cast workaround for the emitterContainer. This is caused by pixi-particles using v6 type for Container so assigning a v5 Container with a different signature does not work. The underlying types work though since I think v6 has only made some previously hidden internals visible due to a different process for generating the type definitions.

This is a bit "hacky" but I suppose it's still worthwhile to keep v5 as some sort of backwards compatibility.

image

pixi-v6-module

I got the v6 module test working with importing the full pixi.js v6 package. I tried changing it to use the individual packages but that turned out to be a bigger challenge with the tsconfig path overrides, yarn workspace no-hoist configuration and the override-require implementation. I'm not sure whether it's worth converting the test to use the individual packages since the functionality of the code should remain the same - just with different imports.

image

pixi-v6-iife

The v6 iife version is where things get interesting: PIXI v6 no longer has ambient declarations so it's not really possible to make the types work simply by adding /// <reference ... to them. So to make the tests work I basically had to remove all PIXI and pixi-particles types from them. I think this test is still good to have since this is probably the way people without complex TypeScript build pipelines use it.

Since v6 no longer really supports the ambient declarations they could probably be completely removed from the package if v5 support is dropped.

image

@qtiki
Copy link
Contributor Author

qtiki commented Jun 14, 2021

@andrewstart what do you think about dropping the v5 support? The types don't really work so it's a bit of a hack to use it with TypeScript. Or should there be a mention in the readme that modules and types only work for v6, and v5 and v6 bundle don't have type support?

It might be cleaner if the package only supported v6 and the ambient types were dropped completely.

@andrewstart
Copy link
Collaborator

Dropping v5 support and ambient types would be fine. It would be nice to get the pixi-v6-module test working with the individual modules, just to make sure that we always work with the bare minimum of packages. The package management should be a bit easier with only one version to keep track of.

@qtiki
Copy link
Contributor Author

qtiki commented Jun 19, 2021

The v6 module test now works with the individual packages. I tested it so that you can delete node_modules/@pixi from the root dir and the test still works. For that to work I had to add some "dependencies of dependencies" also to the package.json. But at least it's very nicely isolated now so you could for example make copies of the test and test it with different minor versions of PIXI.

Also the v6 tests and ambient types are gone.

@qtiki
Copy link
Contributor Author

qtiki commented Jun 19, 2021

Seems I was a bit enthusiastic about removing pixi.js and accidentally broke the examples. They should work again now...

"electron": "^9.4.0",
"override-require": "^1.1.1",
"pixi-particles": "*",
"resource-loader": "3"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like this one is missing its typescript dependency (to get access to the tsc command)

Copy link
Contributor Author

@qtiki qtiki Jun 20, 2021

Choose a reason for hiding this comment

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

Whoa no idea where/when that disappeared. It worked for me but it might be because I have a global typescript package installed. In any case I added it to that test. I also updated all the other typescript dependencies to the latest version and tested that everything still works.

@arathore-gaming
Copy link

Any idea by when this will be done?

@andrewstart
Copy link
Collaborator

Soon-ish. I'm working on another breaking change and want to fold them into one update.

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.

3 participants