emberjs / ember.js

Ember.js - A JavaScript framework for creating ambitious web applications
https://emberjs.com
MIT License
22.46k stars 4.21k forks source link

[Bug] curly invocation of link-to #19730

Open chriskrycho opened 3 years ago

chriskrycho commented 3 years ago

🐞 Describe the Bug

On 3.27.2 and 3.28.0, creating a curried instance of LinkTo with the {{component}} helper and then invoking it as a curly component with a positional argument for the route (rather than the named @route= argument) throws an error.

πŸ”¬ Minimal Reproduction

In the application template:

{{#let (component "link-to") as |link|}}
  {{#link "application"}}go home{{/link}}
{{/let}}

(demo repo here)

πŸ˜• Actual Behavior

This will result in the following errors:

Error occurred:

- While rendering:
  -top-level
    application
      link-to
        -link-to
Uncaught Error: Assertion Failed: You must provide at least one of the `@route`, `@model`, `@models` or `@query` argument to `<LinkTo>`.

πŸ€” Expected Behavior

Curried {{link-to}} invocations should still work through 3.28, as this behavior is not deprecated until 4.0.

🌍 Environment

βž• Additional Context

We may not want to waste time fixing this. It's technically a breaking change, but the fix is trivial, forward-compatible for 4.0 (and indeed unblocks migration to 4.0!), and it's not clear whether fixing it on the source side would be worth the hassle.

I'm mostly opening this just so that if anyone else hits it when upgrading, they have something to see in search results etc.!

runspired commented 2 years ago

This bug may be a bigger issue than stated above, because the currying of params is broken even for the initial dynamic component creation, not just the invocation. This breaks in 3.28+ (possibly before, we moved from 3.24) we experience this issue when creating a component with any dynamic params.

For instance this worked before:

{{#let (component "link-to" "home") as |link|}}
  {{#link}}go home{{/link}}
{{/let}}
chriskrycho commented 2 years ago

Yeah, when I investigated this originally, I found that because of the special handling for at least this, but maybe also a few of the other built-ins, the curried form specifically doesn't work at all with the positional params. The strict mode refactors unfortunately missed this case. My own POV is that it technically should be fixed for the 3.28 LTS if possible… but from conversations with @pzuraq it was non-trivial to do so, and given that folks also have to update to non-positional params immediately anyway to be able to move forward from 3.28, just recommending that they do so seems the best path forward. 😩

runspired commented 2 years ago

If anyone else hits this, I have a codemod that fixes these.