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

Updating WebGLRenderingContext for node #8842

Merged
merged 6 commits into from Nov 15, 2022
Merged

Conversation

michaeljherrmann
Copy link
Contributor

@michaeljherrmann michaeljherrmann commented Nov 10, 2022

Description of change

I'm following up on this PR as the ideal fix we wanted required some updates to some dependencies. I've got those updates merged in gl and for it's types.

Pre-Merge Checklist
  • Tests and/or benchmarks are included
  • Documentation is changed or added
  • Lint process passed (npm run lint)
  • Tests passed (npm run test)

@@ -144,11 +144,11 @@
"@pixi/spritesheet": "7.1.0-alpha",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if I regenerated the lock file correctly? seems to have more changes than I expected. I just ran npm install in the top level after updating the package.json for node

Copy link
Member

Choose a reason for hiding this comment

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

I tried npm i gl@6.0.1 @types/gl@6.0.1 in bundles/pixi.js-node and got the same result, so I think it is OK. Possibly it is caused by the dependency change of gl.

Copy link
Member

Choose a reason for hiding this comment

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

@bigtimebuddy by the way, should we have a npm audit fix for 7.1.0?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, we tried an audit awhile ago (#8480), but kept being blocked by various things. I'm open to doing this again.

@michaeljherrmann
Copy link
Contributor Author

I'm guessing @Zyie should probable review this?

@SuperSodaSea
Copy link
Member

By the way, I'm making a refactor on NodeCanvasElement, will follow up this PR.

@michaeljherrmann michaeljherrmann marked this pull request as draft November 10, 2022 22:15
@michaeljherrmann
Copy link
Contributor Author

I think I need to fix some more typings still in DefinitelyTyped

@michaeljherrmann michaeljherrmann marked this pull request as ready for review November 12, 2022 02:51
@bigtimebuddy bigtimebuddy added this to the v7.1.0 milestone Nov 12, 2022
@SuperSodaSea
Copy link
Member

SuperSodaSea commented Nov 12, 2022

By the way, it seems that the extension interfaces like STACKGL_resize_drawingbuffer and STACKGL_destroy_context are not exported in @types/gl? I am trying to use STACKGL_resize_drawingbuffer and got Module '"gl"' has no exported member 'STACKGL_resize_drawingbuffer'. If they can be exported we can have a better typing.

Trying to make a PR for DefinitelyTyped...

@SuperSodaSea
Copy link
Member

@michaeljherrmann Now that my PR for @types/gl is merged, would you like to bump it to 6.0.2?

@michaeljherrmann
Copy link
Contributor Author

@michaeljherrmann Now that my PR for @types/gl is merged, would you like to bump it to 6.0.2?

@SuperSodaSea done!

Copy link
Member

@SuperSodaSea SuperSodaSea left a comment

Choose a reason for hiding this comment

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

Thanks!

@SuperSodaSea SuperSodaSea 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 Nov 15, 2022
@bigtimebuddy bigtimebuddy merged commit f8e70ac into pixijs:dev Nov 15, 2022
@bigtimebuddy
Copy link
Member

Thank you @michaeljherrmann and @SuperSodaSea!

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

3 participants