Skip to content

render function isn't checking the default property for the constructor #32

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

Closed
pngwn opened this issue Jun 9, 2019 · 2 comments · Fixed by #34
Closed

render function isn't checking the default property for the constructor #32

pngwn opened this issue Jun 9, 2019 · 2 comments · Fixed by #34
Labels

Comments

@pngwn
Copy link

pngwn commented Jun 9, 2019

I'm in the process of writing some testing examples and documentation for the Svelte community and was hoping to use the testing-library DOM helpers to do so, they have quickly become the gold standard for DOM testing in a node environment and I'd love to recommend them. However, I've come across a problem with the way the render method currently works.

Currently render expects the component constructor to appear as the default export on the compiled output but this is only true when compiling to esm or using a bundler. When compiling to cjs via svelte.compile the component constructor is actually on the default property of the compiled output object. This is because cjs doesn't allow named and default exports like esm does, so Svelte has adopted an, admittedly strange, alternative. Here is an example of the output when compiling to cjs.

The reason this hasn't been a problem in your internal tests is because you are using a Jest transform that uses rollup to bundle inside the transform (this might be problematic too for many users, but I'll make another issue to discuss that).

The result of this is that transforms or compilation methods that use the svelte API rather than using a bundler will be confusing to users since they would need to do something like:

import App from './App.svelte';

const { getByText } = render(App.default);

I think it would be worth ensuring the users expectations are met here since the fix is very simple. We just need to check to see if the default property exists on the Component object before using Component directly. I have been using a modified render function that solves this like so:

const ComponentConstructor = Component.default || Component;

const component = new ComponentConstructor({
  ...options,
  target,
});

If you're interested in this change, I would be happy to PR it.

Thanks again for creating this Svelte version of testing-library, the documentation and community are great and it gives Svelte users a great option for testing their components which we have been lacking for some time.

@benmonro
Copy link
Member

benmonro commented Jun 9, 2019

Happy to review a PR for this, and thank you in advance for your contribution

@benmonro
Copy link
Member

🎉 This issue has been resolved in version 1.7.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging a pull request may close this issue.

2 participants