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

{{ensure-safe-component}} not working with template tags #1382

Closed
simonihmig opened this issue Mar 28, 2023 · 5 comments
Closed

{{ensure-safe-component}} not working with template tags #1382

simonihmig opened this issue Mar 28, 2023 · 5 comments

Comments

@simonihmig
Copy link
Collaborator

We have an interoperability issue here:

  • the resolver-transform checks for the existence ensure-safe-component helper by a simple string comparison, allowing for the helper to be used only when having exactly that dasherized name
  • in a template tag environment, we need to import the helper instead of referring to it by that global string reference. But we cannot import it or alias it using that name, as dashes are not valid identifiers in JS scope

Note: in my use case there is currently no way to not use ensure-safe-component, as I need currying and the curried component is not resolvable using a string. And we don't have a variant of the component helper that does only the currying, without the string lookup (which would then be safe under Embroider)...

Reproduction in this PR: https://github.com/CrowdStrike/ember-headless-form/pull/84/files#diff-e61a0d763a79e50ad7c8eb97240e695edaacc3dde08a2f7669013a45a544f709R527.
Works fine in classic, but Embroider fails with Unsafe dynamic component: cannot statically analyse this expression as it does not recognise the camelcased reference.

@NullVoxPopuli
Copy link
Collaborator

Why use this?

I mean, it def should work, but if you need to be unblocked for now, i wouldn't say you need it. Since Field component is just an assignment

https://github.com/CrowdStrike/ember-headless-form/pull/84/files#diff-e61a0d763a79e50ad7c8eb97240e695edaacc3dde08a2f7669013a45a544f709R195

You could?:

        (hash
          Field=(component
            FieldComponent
            data=this.etc

Though, maybe i'm missing something?

@simonihmig
Copy link
Collaborator Author

Why use this?

This was needed to fix the linting issue you also saw in your PR, due to the yielded component having a generic type (the DATA of the field needs to be the same as that of the parent form).

But this is not even the problem here. Even without it we had that, as you observed yourself. So Embroider will only consider (component "some-string") safe, but (component FieldComponent) and (component this.FieldComponent) are equally considered unsafe, AFAIK.

@NullVoxPopuli
Copy link
Collaborator

🙃
I'll have a go at a fix today

@NullVoxPopuli
Copy link
Collaborator

Attempt at test here: #1383
if anything looks off, lemme know

@simonihmig
Copy link
Collaborator Author

I can confirm #1383 successfully fixes this! 🎉

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