-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Migrate to custom elements v1 API #4167
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
Conversation
|
nice! testing on my vrland.io project and it works without any problem |
|
I think this is ready to go. Tested on Edge (42), Firefox Desktop (66), Chrome Desktop (m74), Safari and Chrome for iOS 12.2, Chrome for Android (m74), Oculus Browser, Supermedium and Exokit. Only known compatibility issue is with old IE due to the lack of JS classes enforced by custom Elements V1. |
|
There's a travis failing test that I don't understand. Not sure what |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hope you don't mind the drive-by review. Just commenting on WC v1 and ES6 things.
This is awesome to see :)
|
@WebReflection @justinfagnani I incorporated your feedback. Thanks a lot. That's very generous of your time, it's a big PR 👍 |
Co-Authored-By: Justin Fagnani <[email protected]>
src/core/a-mixin.js
Outdated
| }, | ||
| class AMixin extends ANode { | ||
| constructor () { | ||
| super(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is ANode is extending HTMLElement, same self = super(self) dance might apply
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think you're commenting in an outdated snippet:
Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did, apologies for the confusion
|
This is awesome! I'd started an experimental project to try porting A-Frame to Typescript + webpack, but soon realised that upgrading to custom elements v1 was necessary first. I was thinking to use esmodule imports instead of require too.
Could we make multiple UMD dist targets for es3, es5 etc with webpack? Not sure how keen the community would be on Typescript, but happy to have a go if the consensus is that it would increase robustness. My setup generates the TS definitions too, so you don't need to maintain them separately. |
|
It was a bit more complicated than I thought, but I've got a cross-browser ts + webpack + custom elements setup working here: https://github.com/edsilv/aframe-ts-webpack |
|
@dmarcos i have this branch running since some time without any problems, but now some trouble with exokit the exoKit dev means this is a problem with this branch |
|
☝️ I found the above crash occurs anytime |
|
can this be updated to latest master? |
FYI the latest, better, improved, polyfill, targeting IE11 and every other browser, is this one: P.S. worth mentioning this polyfill has no constructor caveats, like the previous one had 👋 |
|
@arpu Not a top priority at the moment. Any particular needs? |
|
@dmarcos nothing special want to look how i can convert to jsm threejs |
|
replaced by #5136 |
|
close in favor of #5136 |
Migrate all A-Frame elements to custom elements V1 API. Both Firefox and Chrome ship implementations today. Chrome is soon deprecating the old
document.registerElement. This PR will make A-Frame less reliant on the polyfill, behavior more consistent across browsers (#4164) and likely will come with perf improvements as well.These changes touch most of the lines in a bunch of files. It would be good to merge at the beginning of the release cycle to have time to work out the kinks before 1.0. Examples work fine in both desktop Chrome (m74) and Firefox (66) as is.
The main drawback is that Custom Elements v1 enforces JS classes that are not available in older browsers.
Pending work: