Skip to content

Fix Spector.js on Firefox by not initializing polyfill WebXR classes #285

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Sep 11, 2023

Conversation

dmliao
Copy link
Contributor

@dmliao dmliao commented Sep 11, 2023

…until we initialize WebXR capture.

#257 enabled WebXR support for Spector.JS on Chrome, but broke Firefox because the polyfilled experimental WebXR APIs weren't implemented in Firefox, and module importing was trying to load them even if they were never being used.

This change fixes Spector.JS on Firefox by ensuring that the polyfilled WebXR APIs aren't even defined until Spector.JS is called with enableXRCapture, so using it by default on Firefox will no longer cause problems. Note that enableXRCapture still won't work on Firefox, however.

Caveats

While this is the simplest fix I could think of, it currently does not pass tslint, because of a limitation of 1 class per file. However, the current tsconfig also does not support dynamic imports (nor does the webpack configuration, I believe), which makes it otherwise difficult to ensure we don't try to access the WebXR APIs until they're needed in initialization.

(We can probably make an alternate version of this fix by polyfilling the WebXR apis without using classes, which will be more complex.)

Testing

I've verified that the samples once again work on Firefox, and I tested the WebXR samples against the Immersive Web Emulator on Chrome. I don't currently have access to a headset to do testing there.

@dmliao
Copy link
Contributor Author

dmliao commented Sep 11, 2023

I have an alternate solution at dmliao@3a2e3b0

It has the similar principle of not defining modified WebXR API classes until WebXR is initialized, but keeps the classes in their own files (and also runs afoul of the one-class-per-file tslint rule, though that one uses class expressions only). Personally, I think either is fine, but you may have a stronger opinion on which style is better.

Either way, WebXR capture won't be supported in Firefox, and it'll continue to fail at initialization. I can look deeper into that later if it'd be important to figure out.

Appendix

I've tried a number of other approaches, most of which were dead ends:

  • Using ES5 prototype chaining fails when using the webxr-polyfill, because that polyfill defines the WebXR API as ES6 classes
  • MDN suggests composition rather than inheritance for modifying native APIs, but this also fails with webxr-polyfill, which checks against private variables on created WebXR layers (and other objects), which would not be visible on the parent-composition object.

Copy link
Member

@sebavan sebavan left a comment

Choose a reason for hiding this comment

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

looks all good to me, np for ts-lint

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants