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

Minimal implementation to HTMLElement interface #44

Closed
wants to merge 1 commit into from

Conversation

tbranyen
Copy link

Please don't close because this code sucks. I'm hoping to get folks talking about what integration could look like for Custom Elements / Shadow DOM in jsdom. I really like jsdom and it's a good tool for me to get my testing work done. This is one of the de-facto DOM implementations in Node and it would be a huge win to the Web Components community if we could get them working in jsdom.

I am a complete newb to the source, so I can't shake the feeling I'm doing everything wrong and would like feedback on how to improve it.

This is also not anywhere's close to feature complete. I would need to walk through the specification and ensure that everything works to a reasonable degree.

@@ -111,9 +111,21 @@ Interface.prototype.generateConstructor = function () {
this.str += `
iface.setup(this${passArgs});
}\n`;
} else if (this.name === 'HTMLElement') {
this.str += `function ${this.name}() {
const { document, customElements } = global.window;
Copy link
Author

Choose a reason for hiding this comment

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

Chrome will auto-detect the environment; I don't know how to do this in jsdom. I'm cheating and using global.window which would be set like global.window = jsdomInstance.window. This is really bad and I want to have it infer directly from jsdom.

This is required since you cannot call new on a subclassed HTMLElement that has not been registered with the active CustomElementsRegistry.

Copy link

Choose a reason for hiding this comment

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

hmm, I've run into this myself. I ended up having to add so much crap to simulate the window object I created its own module and would pull it in as a dependency

@Sebmaster
Copy link
Member

webidl2js is only used for converting IDL interfaces to JS. It does not contain any actual logic for implementation, that should all go into jsdom itself.

@Sebmaster Sebmaster closed this May 29, 2017
@@ -132,6 +144,10 @@ Interface.prototype.generateRequires = function () {

requireStr += `const impl = utils.implSymbol;\n`;

if (this.name === 'HTMLElement') {
requireStr += `const registry = utils.registrySymbol;\n`;
Copy link
Author

Choose a reason for hiding this comment

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

This is used by the changes I've made in jsdom core.

@tbranyen
Copy link
Author

@Sebmaster Is that possible? This literally writes out the constructor, not jsdom.

@Sebmaster
Copy link
Member

This just wraps the constructor, validates params and calls the jsdom implementation constructor.

@tbranyen
Copy link
Author

No it writes out code that says: throw new TypeError("Illegal constructor"); how can JSDOM correct this? It is set in stone once the module evaluates.

@tbranyen
Copy link
Author

tbranyen commented May 29, 2017

Just so you have reference, I'm trying to make this work:

const { equal } = require('assert');
const { JSDOM } = require('jsdom');
const { window } = new JSDOM();
const { HTMLElement, customElements } = window;

// Unfortunately this is required with my implementation.
global.window = window;

class CustomElement extends HTMLElement {
  constructor(props) {
    super();

    this.innerHTML = `<span>${props.someMessage}</span>`;
  }
}

customElements.define('custom-element', CustomElement);

const domNode = new CustomElement({ someMessage: 'Hello world' });
equal(domNode.outerHTML, '<custom-element><span>Hello world</span></custom-element>');

@Sebmaster
Copy link
Member

Oh I see.

I think my preferred solution to this would be wrapping the webidl2js exported interface another time in jsdom. @domenic ?

tbranyen added a commit to tbranyen/jsdom that referenced this pull request May 29, 2017
Changes here paired with jsdom/webidl2js#44 will
enable `./test-web-components.js` to work. You will need to `npm
install` before running `node ./test-web-components.js`.
@tbranyen
Copy link
Author

That sounds good to me. Once I have a blessed path for integration, I'll fill out the rest of the specification.

@domenic
Copy link
Member

domenic commented May 29, 2017

The spec takes care of this with the [HTMLConstructor] extended attribute. We need to have either jsdom or webidl2js implement that. Not sure the best way to do that; perhaps some kind of plugin system for webidl2js to mimic how the HTML Standard plugs in to the Web IDL spec by defining its own extended attribute.

In any case, when implementing be sure to follow the complete spec for [HTMLConstructor].

@Sebmaster
Copy link
Member

Yeah, guess the plugin system will have to be it (can't have webidl2js access some global), although I have no idea how we'd implement that to modify the stringified code...

stevedorries pushed a commit to stevedorries/webidl2js that referenced this pull request Jan 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants