Skip to content

Conversation

@dmarcos
Copy link
Member

@dmarcos dmarcos commented May 13, 2019

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:

@arpu
Copy link
Contributor

arpu commented May 15, 2019

nice! testing on my vrland.io project and it works without any problem
testing on linux chrome 74 fedora 30

@dmarcos
Copy link
Member Author

dmarcos commented May 16, 2019

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.

@dmarcos
Copy link
Member Author

dmarcos commented May 16, 2019

There's a travis failing test that I don't understand. Not sure what mocha --ui tdd tests/node tests and why it's failing on the polyfill or how to fix it.

Copy link

@justinfagnani justinfagnani left a 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 :)

@dmarcos
Copy link
Member Author

dmarcos commented May 17, 2019

@WebReflection @justinfagnani I incorporated your feedback. Thanks a lot. That's very generous of your time, it's a big PR 👍

@dmarcos dmarcos force-pushed the customElementsV1 branch from 299cd35 to 8931049 Compare May 17, 2019 18:17
@dmarcos dmarcos force-pushed the customElementsV1 branch from c47b2c5 to 1e0a25d Compare May 17, 2019 18:47
},
class AMixin extends ANode {
constructor () {
super();

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

Copy link
Member Author

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:

https://github.com/aframevr/aframe/pull/4167/files#diff-631b4510fca3a68661ae2833c57e1e3dR18

Thanks

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

@edsilv
Copy link
Contributor

edsilv commented Jun 1, 2019

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.

The main drawback is that Custom Elements v1 enforces JS classes that are not available in older browsers.

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.

@edsilv
Copy link
Contributor

edsilv commented Jun 2, 2019

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

@arpu
Copy link
Contributor

arpu commented Jun 28, 2019

@dmarcos i have this branch running since some time without any problems, but now some trouble with exokit
same problem accourse if in chrome if aframe load script is set to the end of the html

the exoKit dev means this is a problem with this branch
testing on this build url https://vrland.io/exo_customElementsV1.html

(node:19816) Warning: Setting the NODE_TLS_REJECT_UNAUTHORIZED environment variable to '0' makes TLS connections and HTTPS requests insecure by disabling certificate verification.
(node:19816) Warning: Setting the NODE_TLS_REJECT_UNAUTHORIZED environment variable to '0' makes TLS connections and HTTPS requests insecure by disabling certificate verification.
THREE.WebGLRenderer 103dev
ErrorEvent {
  type: 'error',
  target: <script src="aframe_customElementsV1.js"></script>,
  bubbles: false,
  cancelable: false,
  defaultPrevented: false,
  propagationStopped: false,
  currentTarget: <script src="aframe_customElementsV1.js"></script>,
  message: 'systems[name] is not a constructor',
  stack: 'https://vrland.io/aframe_customElementsV1.js:76834\n    ' +
    'this.systems[name] = new systems[name](this);\n                        ' +
    ' ^\n\nTypeError: systems[name] is not a constructor\n    at ' +
    'AScene.initSystem ' +
    '(https://vrland.io/aframe_customElementsV1.js:76834:26)\n    at ' +
    'AScene.initSystems ' +
    '(https://vrland.io/aframe_customElementsV1.js:76821:10)\n    at ' +
    'AScene.connectedCallback ' +
    '(https://vrland.io/aframe_customElementsV1.js:76787:10)\n    at ' +
    'CustomElementRegistry.upgrade ' +
    '(/home/arpu/Work/githubsources/exokit/src/Window.js:285:38)\n    at ' +
    '/home/arpu/Work/githubsources/exokit/src/Window.js:210:14\n    at ' +
    '_recurse (/home/arpu/Work/githubsources/exokit/src/DOM.js:1305:22)\n   ' +
    ' at _recurse ' +
    '(/home/arpu/Work/githubsources/exokit/src/DOM.js:1311:28)\n    at ' +
    '_recurse (/home/arpu/Work/githubsources/exokit/src/DOM.js:1311:28)\n   ' +
    ' at _recurse ' +
    '(/home/arpu/Work/githubsources/exokit/src/DOM.js:1311:28)\n    at ' +
    'Document.traverse ' +
    '(/home/arpu/Work/githubsources/exokit/src/DOM.js:1322:12)'
}
https://vrland.io/aframe_customElementsV1.js:76834
    this.systems[name] = new systems[name](this);
                         ^

TypeError: systems[name] is not a constructor
    at AScene.initSystem (https://vrland.io/aframe_customElementsV1.js:76834:26)
    at AScene.initSystems (https://vrland.io/aframe_customElementsV1.js:76821:10)
    at AScene.connectedCallback (https://vrland.io/aframe_customElementsV1.js:76787:10)
    at CustomElementRegistry.upgrade (/home/arpu/Work/githubsources/exokit/src/Window.js:285:38)
    at /home/arpu/Work/githubsources/exokit/src/Window.js:210:14
    at _recurse (/home/arpu/Work/githubsources/exokit/src/DOM.js:1305:22)
    at _recurse (/home/arpu/Work/githubsources/exokit/src/DOM.js:1311:28)
    at _recurse (/home/arpu/Work/githubsources/exokit/src/DOM.js:1311:28)
    at _recurse (/home/arpu/Work/githubsources/exokit/src/DOM.js:1311:28)
    at Document.traverse (/home/arpu/Work/githubsources/exokit/src/DOM.js:1322:12)

@avaer
Copy link

avaer commented Jun 28, 2019

☝️ I found the above crash occurs anytime <a-scene> is seen by the aframe.js initial tick -- e.g. if the script is at the end of the </body>. That behavior is not specific to Exokit.

@arpu
Copy link
Contributor

arpu commented Sep 4, 2020

can this be updated to latest master?

@WebReflection
Copy link

WebReflection commented Sep 4, 2020

can this be updated to latest master?

FYI the latest, better, improved, polyfill, targeting IE11 and every other browser, is this one:
https://github.com/WebReflection/custom-elements#readme

P.S. worth mentioning this polyfill has no constructor caveats, like the previous one had 👋

@dmarcos
Copy link
Member Author

dmarcos commented Sep 4, 2020

@arpu Not a top priority at the moment. Any particular needs?

@arpu
Copy link
Contributor

arpu commented Sep 6, 2020

@dmarcos nothing special want to look how i can convert to jsm threejs

@vincentfretin
Copy link
Contributor

replaced by #5136

@dmarcos
Copy link
Member Author

dmarcos commented Oct 25, 2022

close in favor of #5136

@dmarcos dmarcos closed this Oct 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants