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

v3.0.7 TypeScript types broken #402

Open
qtiki opened this issue Aug 5, 2021 · 18 comments
Open

v3.0.7 TypeScript types broken #402

qtiki opened this issue Aug 5, 2021 · 18 comments

Comments

@qtiki
Copy link

qtiki commented Aug 5, 2021

Types seem to be broken in v3.0.7 resulting in the following error:

node_modules/@pixi-spine/runtime-3.8/index.d.ts:7:41 - error TS2307: Cannot find module '@pixi-spine/base/compile' or its corresponding type declarations.

7 import { IAnimationStateListener } from '@pixi-spine/base/compile';
                                          ~~~~~~~~~~~~~~~~~~~~~~~~~~

As the error says the runtime-3.8/index.d.ts file has an incorrect import from @pixi-spine/base/compile. Changing this to @pixi-spine/base manually seems to work as expected.

@ivanpopelyshev
Copy link
Collaborator

oh !@#$ gonna fix it right now

@qtiki
Copy link
Author

qtiki commented Aug 5, 2021

I cloned the repo to look into this but I ran into other weird issues. For example the base package has the peer dependencies defined like this:

    "@pixi/constants": "^6.0.2",
    "@pixi/core": "^6.0.2",
    "@pixi/display": "^6.0.2",
    "@pixi/graphics": "^6.0.2",
    "@pixi/math": "^6.0.2",
    "@pixi/mesh-extras": "~6.0.2",
    "@pixi/sprite": "~6.0.2",
    "@pixi/utils": "~6.0.2"

So the mesh-extras, sprite and utils seem to be fixed to a patch release whereas the other packages are fixed to a minor release. This probably causes problems with the new PIXI 6.1.0 release.

@qtiki
Copy link
Author

qtiki commented Aug 5, 2021

Not sure what's causing the problems I'm having but it seems some PIXI packages are version 6.0.4 and some 6.1.0 which causes all sorts of type incompatibilities when trying to build the project with rush. I think the way the dependencies were all changed to peer dependencies in PIXI 6.1.0 will probably need some more changes to work correctly.

@ivanpopelyshev
Copy link
Collaborator

ivanpopelyshev commented Aug 5, 2021

Oh , right, 6.1.0 is out, means i have to change everything because loader typings might be different now .

@ivanpopelyshev
Copy link
Collaborator

@qtiki

Also, i have to use ^, right?

@qtiki
Copy link
Author

qtiki commented Aug 5, 2021

I think ^ is more common. It allows for minor updates, ie. 6.x.x whereas ~ allows only patch releases, ie. 6.0.x - https://docs.npmjs.com/about-semantic-versioning

However according to strict semantic versioning the amount of breaking changes in PIXI v6.1.0 means it should probably have been a v7 release. I'd still say use the caret ^ and let's hope no more breaking changes are coming.

Regardless whether you use ^ or ~ all of the @PIXI dependencies should use the same one to ensure they're always using the same version.

@ivanpopelyshev
Copy link
Collaborator

Can you please check 3.0.8 right now?

@qtiki
Copy link
Author

qtiki commented Aug 5, 2021

3.0.8 seems to work. At least it compiles without errors. Can't really test it further than that yet because there's a bunch of breaking changes in PIXI 6.1.0 that I need to sort out in our own stuff before that.

@ivanpopelyshev
Copy link
Collaborator

yes, ILoaderResource->LoaderResource in types
tell me which changes were breaking for you?

@qtiki
Copy link
Author

qtiki commented Aug 5, 2021

We have only added the packages that we directly use as dependencies which previously were installed automatically so basically all the stuff that was moved to peer dependencies needs to be added. At the moment those result in a bunch of import errors.

Also we have some problems with the extremely hacky pixi-particles "release" that we use - which is basically a GitHub fork from my PR and not a real NPM package since there's no official pixi-particles release for PIXI v6 yet. That'll need some work because of the renaming of the @pixi/particles package. Also that fork should probably be updated to use the peer dependencies like PIXI packages.

And while doing all this I ran into a new issue with 6.1.0: pixijs/pixijs#7674

There might be more but I need to fix some stuff before I know for sure.

@qtiki
Copy link
Author

qtiki commented Aug 5, 2021

Seems that there is also a bunch of errors from @pixi/sound. Not sure if it's even supposed to support 6.1.0 yet. I'll dig deeper and open issues there if needed.

@ivanpopelyshev
Copy link
Collaborator

ivanpopelyshev commented Aug 5, 2021

sound? same, Loader types :) Gonna poke @bigtimebuddy about that
particles? yes, that's one of a heck change

@bigtimebuddy
Copy link
Collaborator

I didn't have any problem with @pixi/[email protected] and [email protected] playing together without errors. Here's the example: https://github.com/bigtimebuddy/pixi-sound-webpack-example but the upgrade process was definitely clunky. I had to nuke the node_modules and lock.

@qtiki
Copy link
Author

qtiki commented Aug 6, 2021

I'm seeing these weird errors with @pixi/sound after updating:

node_modules/@pixi/sound/dist/pixi-sound.d.ts:384:5 - error TS2416: Property 'init' in type 'HTMLAudioInstance' is not assignable to the same property in base type 'IMediaInstance'.
  Type '(media: HTMLAudioMedia) => void' is not assignable to type '(parent: IMedia) => void'.
    Types of parameters 'media' and 'parent' are incompatible.
      Type 'IMedia' is missing the following properties from type 'HTMLAudioMedia': parent, _source, source, eventNames, and 9 more.

384     init(media: HTMLAudioMedia): void;
        ~~~~

I tried also nuking the node_modules and lockfile without any change. It's probably something weird related to our setup. I know this is not the right issue nor the right project even but since this discussion sort of got derailed I'll just comment here.

I'll dig in deeper tomorrow to see if I can isolate what's causing this.

@qtiki
Copy link
Author

qtiki commented Aug 6, 2021

Actually it seems that we had @pixi/sound explicitly set to 4.0.3 before I updated PIXI. Meaning that this has nothing to do with the PIXI 6.1.0 release but is a specific error in @pixi/sound 4.0.4.

Aaaand also it seems that I have already opened an issue about this earlier (that I completely forgot about because I was just on a two week vacation to reset my brain): pixijs/sound#165

@ivanpopelyshev
Copy link
Collaborator

Oh its strict stuff, interesting. I leave issue open so people can see it

@FloodGames
Copy link

Is this related? Perhaps someone can fix it for you.

➤ YN0002: │ pixi-spine@npm:3.0.13 doesn't provide @pixi/app (pd0e0f), requested by @pixi-spine/loader-base
➤ YN0002: │ pixi-spine@npm:3.0.13 doesn't provide @pixi/constants (pa45c0), requested by @pixi-spine/base
➤ YN0002: │ pixi-spine@npm:3.0.13 doesn't provide @pixi/constants (pf7296), requested by @pixi-spine/loader-base
➤ YN0002: │ pixi-spine@npm:3.0.13 doesn't provide @pixi/constants (p288ec), requested by @pixi-spine/runtime-3.7
➤ YN0002: │ pixi-spine@npm:3.0.13 doesn't provide @pixi/constants (p5f452), requested by @pixi-spine/runtime-3.8
➤ YN0002: │ pixi-spine@npm:3.0.13 doesn't provide @pixi/constants (p4ac49), requested by @pixi-spine/runtime-4.0
➤ YN0002: │ pixi-spine@npm:3.0.13 doesn't provide @pixi/core (pa3ea4), requested by @pixi-spine/base
➤ YN0002: │ pixi-spine@npm:3.0.13 doesn't provide @pixi/core (p1af53), requested by @pixi-spine/loader-base
➤ YN0002: │ pixi-spine@npm:3.0.13 doesn't provide @pixi/display (p8fb1b), requested by @pixi-spine/base
➤ YN0002: │ pixi-spine@npm:3.0.13 doesn't provide @pixi/display (pbf5ea), requested by @pixi-spine/loader-base
➤ YN0002: │ pixi-spine@npm:3.0.13 doesn't provide @pixi/graphics (pcfa6d), requested by @pixi-spine/base
➤ YN0002: │ pixi-spine@npm:3.0.13 doesn't provide @pixi/loaders (pba701), requested by @pixi-spine/loader-base
➤ YN0002: │ pixi-spine@npm:3.0.13 doesn't provide @pixi/loaders (p062dd), requested by @pixi-spine/loader-uni
➤ YN0002: │ pixi-spine@npm:3.0.13 doesn't provide @pixi/math (p821eb), requested by @pixi-spine/base
➤ YN0002: │ pixi-spine@npm:3.0.13 doesn't provide @pixi/math (p29a12), requested by @pixi-spine/runtime-3.7
➤ YN0002: │ pixi-spine@npm:3.0.13 doesn't provide @pixi/math (p91c3b), requested by @pixi-spine/runtime-3.8
➤ YN0002: │ pixi-spine@npm:3.0.13 doesn't provide @pixi/math (pe4259), requested by @pixi-spine/runtime-4.0
➤ YN0002: │ pixi-spine@npm:3.0.13 doesn't provide @pixi/mesh (p892e2), requested by @pixi-spine/base
➤ YN0002: │ pixi-spine@npm:3.0.13 doesn't provide @pixi/mesh-extras (pd6912), requested by @pixi-spine/base
➤ YN0002: │ pixi-spine@npm:3.0.13 doesn't provide @pixi/sprite (pf5206), requested by @pixi-spine/base
➤ YN0002: │ pixi-spine@npm:3.0.13 doesn't provide @pixi/utils (p85474), requested by @pixi-spine/base
➤ YN0002: │ pixi-spine@npm:3.0.13 doesn't provide resource-loader (p18ac3), requested by @pixi-spine/loader-base
➤ YN0000: │ Some peer dependencies are incorrectly met; run yarn explain peer-requirements for details, where is the six-letter p-prefixed code

package.json:
"pixi-spine": "^3.0.13",
"pixi.js": "^6.2.0",

@FloodGames
Copy link

Will this also fix the skeleton.setSkin method? (used in combining skins)
image

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

No branches or pull requests

4 participants