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

@embroider/test-setup v2 causes assertion for HTML encoded char to fail #1380

Closed
jelhan opened this issue Mar 27, 2023 · 9 comments
Closed

Comments

@jelhan
Copy link
Contributor

jelhan commented Mar 27, 2023

I noticed a regression if an application renders a HTML encoded character and asserts against the character not being encoded in @embroider/test-setup v2.

I have a component, which renders ×: https://github.com/ember-bootstrap/ember-bootstrap/blob/d6cf431bf2871debf04915967271419a0c848fdb/addon/components/bs-modal/header/close.hbs#L2

I have a test, which asserts assert.dom(this.element).hasText('×');.

The assertion is working fine in non-Embroider builds and with @embroider/test-setup v1. But it starts to fail when upgrading to @embroider/test-setup v2: ember-bootstrap/ember-bootstrap#1857

Asserting against the encoded HTML character works with @embroider/test-setup v2 but does not work with non-Embroider builds. At this point I'm stuck.

@jelhan
Copy link
Contributor Author

jelhan commented Mar 27, 2023

I tried reproducing the issue preventing us from asserting against the HTML encoded character. But I was not able to reproduce it in a simplified setup: https://github.com/jelhan/ember-test-embroider-test-setup-with-html-encoded-char

Nevertheless here is a PR for Ember Bootstrap showing that it fails for that complex addon: ember-bootstrap/ember-bootstrap#1912

I guess there is another thing playing into it...

@simonihmig
Copy link
Collaborator

I believe what has changed in test-setup v2 is only that it will bring in Embroider v2 instead of v1. So likely the issue is related to the new version of Embroider then!

I vaguely remember you had brought up a similar issue before, right? What was the problem then, and the fix?

@jelhan
Copy link
Contributor Author

jelhan commented Mar 28, 2023

I vaguely remember you had brought up a similar issue before, right? What was the problem then, and the fix?

I mentioned that issue some weeks ago on Discord. But haven't had time to create a GitHub issue yet. Noticed it first in Ember Bootstrap around January. Not solved yet. Just haven't had time to get back to it.

@jrjohnson
Copy link
Contributor

I'm seeing this issue with embroider v3 in our embroider tests as well. I'm unable to reproduce it in a simple app, it may have to do with the number of {{outlets}} or something else about nesting because it doesn't happen for things in application.hbs, but starts to happen at some point in the tree. I'm going to keep playing with a test app to see if I can find the failure point, but something is weird here for sure.

@ef4
Copy link
Contributor

ef4 commented May 19, 2023

This looks like a glimmer printer bug. In a quick check, it looks like parsing and re-printing a template does:

-×
+×

If I'm right, this bug happens only in components inside addons, and only when those addons have some custom AST transforms applied. Under those conditions, embroider needs to compile away the custom stuff before the app builds, so it will run the component's template through the template compiler, re-serializing to hbs after transforming.

@ef4
Copy link
Contributor

ef4 commented May 19, 2023

Might be fixed already and we're not using the right setting here: glimmerjs/glimmer-vm#938

@ef4
Copy link
Contributor

ef4 commented May 19, 2023

@ef4
Copy link
Contributor

ef4 commented May 19, 2023

Fixed in babel-plugin-ember-template-compilation 2.0.3. As it's a patch release, no embroider release is needed, just make sure you let your deps float (or patch your lockfile) so you get the update.

@ef4 ef4 closed this as completed May 19, 2023
@jrjohnson
Copy link
Contributor

Thanks @ef4, this fixed the issue for me 🎉

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

4 participants