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

forgo.mount() typing not matching with behaviour #72

Open
chronoDave opened this issue Aug 7, 2022 · 3 comments
Open

forgo.mount() typing not matching with behaviour #72

chronoDave opened this issue Aug 7, 2022 · 3 comments
Assignees

Comments

@chronoDave
Copy link
Contributor

When trying to run the following code I got an unexpected error:

import * as forgo from "forgo";

const App= () =>
  new forgo.Component({
    render() {
      return <p>Tooltip</p>;
    }
  });

forgo.mount(document.getElementById("root"), <App />);

forgo.min.js:520 Uncaught Error: The container argument to the mount() function should be an HTML element.

Now, because of the typing of mount(), this doesn't throw an error, though I'm not sure why.

export function mount(
  forgoNode: ForgoNode,
  container: Element | string | null
): RenderResult {
  return forgoInstance.mount(forgoNode, container);
}

null however is never a valid container because if (parentElement) will always return false.

@chronoDave
Copy link
Contributor Author

chronoDave commented Aug 7, 2022

Sandbox

@spiffytech
Copy link
Member

You have the arguments reversed. This works:

forgo.mount(<App />, document.getElementById("root"));

I'll see if I can tighten up the type signature to blow up correctly.

@spiffytech spiffytech self-assigned this Aug 9, 2022
@spiffytech
Copy link
Member

I'm going to wait on this types change until we publish breaking changes is v4.0. Since document.getElementById() returns type HTMLElement | null, disallowing null would be a breaking change as it would force existing call sites to add a non-null ! assertion.

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

No branches or pull requests

2 participants