Skip to content

Render JSX in t.snapshot arguments #1214

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
vadimdemedes opened this issue Jan 30, 2017 · 11 comments
Closed

Render JSX in t.snapshot arguments #1214

vadimdemedes opened this issue Jan 30, 2017 · 11 comments
Labels
enhancement new functionality

Comments

@vadimdemedes
Copy link
Contributor

vadimdemedes commented Jan 30, 2017

See #1154 (comment). During the development of magic-assert PR, we decided to extract t.jsxEqual from it and land it bit later with a more clear purpose.

I do have one idea for these, but not sure if that can be implemented to be stable enough. We (or community) could write a babel plugin for react/preact/inferno, which detects t.jsxEqual and convert:

t.jsxEqual(<This/>, <That/>);

into:

t.jsxEqual(renderer.create(<This/>).toJSON(), renderer.create(<That/>).toJSON());

to save the user from typing that manually or creating shortcuts (as mentioned in #1175).


Also related #1175

@vadimdemedes vadimdemedes added the enhancement new functionality label Jan 30, 2017
@novemberborn
Copy link
Member

I wonder if we could advocate JSX transforms that just do this for deepEqual. Was there any specific behavior in jsxEqual?

@vadimdemedes
Copy link
Contributor Author

Nope, there wasn't any specific behavior inside t.jsxEqual. I just thought it may be easier for 3rd-party transforms to detect what needs to be transformed, if we had t.jsxEqual.

@jfmengels
Copy link
Contributor

Jut so I'm sure I understand: Why do we wish to have a new jsxEqual assertion method? Is it because doing

t.deepEqual(<This/>, <That/>);

creates an unreadable object that does not look like what you expect to see in the DOM?

@novemberborn
Copy link
Member

I think this comes down to #1175. We can't do a generic jsxEqual because we need to render the JSX somehow, which means choosing a particular implementation (e.g. React). The solution for that was to encourage Babel plugins that would translate the JSX for use with the assertion. But I'm not convinced those plugins need to latch onto t.jsqEqual calls. They can just detect the JSX AST nodes.

@vadimdemedes
Copy link
Contributor Author

@jfmengels I thought to add it, just to clearly indicate for 3rd-party babel plugins what JSX needs to be transformed.

@novemberborn Actually, this issue can be closed, since goal is to not have restrictive preferences and be open & universal to all JSX-based libraries. I should create a new issue describing my thoughts and a possible route we can take.

But I'm not convinced those plugins need to latch onto t.jsqEqual calls. They can just detect the JSX AST nodes.

I don't have experience in that area, so I can't argue with this. If it's just easy as detecting t.jsxEqual, I'm fine with having just t.deepEqual.

One thought: would this confuse users who'd use t.is with JSX too?

@novemberborn
Copy link
Member

Detecting JSX nodes is pretty easy, yeah.

One thought: would this confuse users who'd use t.is with JSX too?

You can't use that to compare objects though, can you?

@vadimdemedes
Copy link
Contributor Author

Nope, but I'm wondering if users will assume t.is is for shallow compare (JSX) and t.deepEqual for deep JSX.

@novemberborn
Copy link
Member

That confusion doesn't seem exclusive to JSX object comparisons.

@vadimdemedes
Copy link
Contributor Author

Yes, also true.

@vadimdemedes vadimdemedes changed the title Add t.jsxEqual Render JSX in t.snapshot Jun 27, 2017
@vadimdemedes
Copy link
Contributor Author

vadimdemedes commented Jun 27, 2017

New developments on this issue. It's now centered around t.snapshot() instead of testing equality of JSX. The idea is to have separate and optional babel plugins, which would transform t.snapshot(<Hello/>) into t.snapshot(reactTestRenderer.create(<Hello/>).toJSON()). Those plugins would only be responsible for injecting the renderer and placing JSX into reactTestRender.create(JSX).toJSON() code and putting it inside t.snapshot() args.

Renderers:

User should be free to choose any test renderer they desire, but in case we already have a babel plugin for it, they can just go with that.

@vadimdemedes vadimdemedes changed the title Render JSX in t.snapshot Render JSX in t.snapshot arguments Jun 27, 2017
@novemberborn
Copy link
Member

We'd be happy to promote plugins written for this purpose, but this isn't something that AVA needs to support by itself.

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

No branches or pull requests

3 participants