jnsmalm / pixi3d

The 3D renderer for PixiJS. Seamless integration with 2D applications.
https://pixi3d.org
MIT License
759 stars 44 forks source link

Implement against PixiJS packages instead of PixiJS bundle #106

Closed bigtimebuddy closed 2 years ago

bigtimebuddy commented 2 years ago

Changes

This PR uses the @pixi/* packages instead of pixi.js (aka "bundle"). Doing this has a few advantages:

Explicitly defines peerDependencies that are required for minimally building against Pixi3d

jnsmalm commented 2 years ago

Hey @bigtimebuddy , after these changes it doesn't work so well using webpack with PixiJS/Pixi3D. When Pixi3D starts, it will use several "Renderer.registerPlugin". The problem is that Pixi3D uses another instance of PixiJS, so those "Renderer.registerPlugin" doesn't affect the PixiJS instance the app is using. Previously this could be fixed by telling webpack it should resolve both of those PixiJS instances at the same path:

alias: {
  "pixi.js": path.resolve(__dirname, 'node_modules/pixi.js'),
} 

Now that doesn't work because Pixi3D is not using import "pixi.js" any longer, instead it uses import "@pixi/core".

You have any idea how this can be resolved?

bigtimebuddy commented 2 years ago

You would need to add entries for all the peerDependencies:

alias: {
  "@pixi/core": path.resolve(__dirname, 'node_modules/@pixi/core'),
  "@pixi/utils": path.resolve(__dirname, 'node_modules/@pixi/utils'),
  // ...
} 

I thought maybe there was a glob way to do this for all packages

Or you could try something like this:

import AliasRegexOverridePlugin from 'alias-regex-webpack-plugin';

const webpackConfig = {
  plugins: [
    new AliasRegexhOverridePlugin(
      /^(@pixi\/([^\/]+))$/,
      '$1/dist/esm/$2.min.js'
    )
  ]
}
goldenratio commented 2 years ago

Hey, will this change be backwards compatible with pixi v5?

bigtimebuddy commented 2 years ago

It should be backward compatible.

bencresty commented 2 years ago

Thanks for helping 👍

Thanks @jnsmalm and @bigtimebuddy for the quick actions!

jnsmalm commented 2 years ago

@bigtimebuddy Hi! I have unfortunatly started to noticed some issues with these changes. I want Pixi3D to work well across as many. different PixiJS versions as possible and these changes makes it harder to do so. Pixi3D can no longer "analyse" what's on the PIXI object and do the right thing depending on PixiJS version.

Specific example. I would like to support the new extension API in 6.5.0 but at the same time want the same Pixi3D version to work with 5.3.0. To use the new API I need to import { extensions } from "@pixi/extensions" which will crash on a PixiJS version < 6.5.0. By using the PixiJS bundle like before I can simply check if PIXI.extensions is available, and if it's not use the older type of registering the loader.

These changes also added some complexity when using bundlers (with more than one PixiJS version installed) as the users config needs to have all @pixi/* in alias instead of only the single "pixi.js".

Are there other use cases for using @pixi/* instead of "pixi.js" than the cases you have stated above in the PR?

bigtimebuddy commented 2 years ago

Another use case for using packages is to support the new Node.js rendering. This is different from the pixi.js bundle.

Can you tell me more why users need to alias in webpack?

jnsmalm commented 2 years ago

Yes, the new node rendering wouldn't be supported. It would be cool to support it but if I want any chance maintaining this project between PixiJS versions and different browsers I think I can live with this sacrifice :-)

So this is my experience after adding the peer dependencies to @pixi/*.

  1. User do npm install pixi.js@6.4.2. After this the node_modules gets populated with @pixi packages and pixi.js package. At this point nothing is inside node_modules/pixi.js/node_modules. The @pixi packages points to 6.4.2 as it should.
  2. User do npm install pixi3d@1.5.0. After this the @pixi packages (6.4.2) gets copied over to node_modules/pixi.js/node_modules and the @pixi packages at node_modules points to 6.5.1 (as it's the latest version which complies with the peer dependency ^6.0.2).
  3. Now the user have two versions of PixiJS installed, so when using a bunder Pixi3D will use the @pixi packages in node_modules pointing to 6.5.1 and the app (using import * as PIXI from "pixi.js") will get the 6.4.2 version.
  4. When this app runs Pixi3D installs itself as a plug-in to 6.5.1 version while the user version (6.4.2) have not clue about Pixi3D so nothing works.

I'm using Node.js 16.15.1 and NPM 8.11.0.

jnsmalm commented 2 years ago

So right after writing this I tested again the same steps and I get a different result. Now 6.5.1 doesn't get downloaded. I have no idea why the behaviour is different right now - maybe something with my machine.

Anyways, the main point is that I won't be able to support different PixiJS versions (for example with the new extensions API) - so it's a bummer. I may have to revert these changes :-(

jnsmalm commented 2 years ago

Don't you just love JS sometimes? The easy part is writing the actual plugin - just wait until you need to support different versions/bundlers/techniques like Node.js/Rollup/Webpack/ESM/CommonJS 💯

bigtimebuddy commented 2 years ago

Want to throw out a few other options:

jnsmalm commented 2 years ago

let me help you make a boilerplate/template, I think you're over complicating this a bit and think we can streamline it for users.

This is a good idea. I wanted to create a package "create-pixi3d-app" which does this, but didn't do it yet.

my preference would be for you to stay on the head release and drop some of the backward compatibility for v5. Leaning into the latest has a lot of benefits and simplifies things for you. You could also maintain older release branches if v5 or v6 < 6.5 is super important to you.

This is not an option I'm afraid, we are using Pixi3D in projects at my current job where we use PixiJS v5. Actually almost no features in PixiJS v6 is used in Pixi3D (until now with extensions) so it doesn't makes much sense requiring that version from the users. There will probably come a day when v5 support is dropped, but I want to keep that as far into the future as possible.

another option is having a compatibility layer for different Pixi versions. Spine plugin kinda has something like this for various spine runtimes.

I have all been thinking about a solution like this, but don't want to introduce a another package for the user. If you know about a way to do this without creating another package, please let me know :-)

bigtimebuddy commented 2 years ago

You can create more than one entry point for importing so you don't have to deal with multiple packages. So something like import * as Pixi3D from "pixi3d/v5";. This is achieved with the Node.js exports field. It's supported by webpack and others and works with typings.

Also how to support extensions, why not something like this:

import * as core from "@pixi/core";
if ('extensions' in core) {
  // use extensions here
}
bigtimebuddy commented 2 years ago

One way you may be able to do the multiple entry points easily, is to use the JSCC plugin for rollup. This would allow you to have conditional compiling in your code. Pixi uses this but only for DEBUG to silence console spam for the release build. But you could use it to support v5, v6, and v6.5 without having to runtime check.

bigtimebuddy commented 2 years ago

@jnsmalm here's an example project using your demo: https://github.com/bigtimebuddy/create-pixi3d-app

I did see a small typing issue with Loader, which could be fixed by adding these types. PixiJS Sound also does this as it returns a sound object on the LoaderResource.

jnsmalm commented 2 years ago

Thanks a lot, I'll explore this further!

jnsmalm commented 2 years ago

@bigtimebuddy I created some sort of compatibility-layer as you suggested, it seems to work fine. I didn't know that the extensions was available in core package, I thought they were in extensions-package only. The thing with having multiple entry points is a good idea as I can continue creating features which could be used across PixiJS versions. I'll work on that later though as it's not really needed at this point. Thanks again.

bigtimebuddy commented 2 years ago

Sounds good. Don't hesitate to reach out if you need help.