[BUGFIX] <LinkTo> should link within the engine when used inside one
#19223
Conversation
chancancode
force-pushed
the
engine-link-to
branch
2 times, most recently
from
ddc82b2 to
9fce35b
Compare
| @@ -875,7 +875,7 @@ moduleFor( | |||
|
|
|||
| ["@test query params don't have stickiness by default between model"](assert) { | |||
| assert.expect(1); | |||
| let tmpl = '{{#link-to "blog.category" 1337}}Category 1337{{/link-to}}'; | |||
| let tmpl = '{{#link-to "category" 1337}}Category 1337{{/link-to}}'; | |||
There was a problem hiding this comment.
So this is one way you can tell this is a breaking change. Do people really use engines without the engines addon though?
There was a problem hiding this comment.
@chancancode I don't think we actually support engines without the ember-engines addon. All of our documentation and guidance on usage of engines has been in context of using the addon (roughly akin to how we now are allowed to assume folks use ember-source through ember-cli).
There was a problem hiding this comment.
@rwjblue and how about don't use engine.route-name anymore? I think it's a drawback for us
| @@ -487,6 +490,13 @@ const LinkComponent = EmberComponent.extend({ | |||
| init() { | |||
| this._super(...arguments); | |||
|
|
|||
| assert( | |||
|
The issues with CI were already fixed, restarting to see if that works. It might require a rebase though. |
chancancode
force-pushed
the
engine-link-to
branch
from
9fce35b to
56af48b
Compare
|
Rebased |
|
Heads up: I haven't had a chance to dig in yet and it's extremely unlikely I will have a chance to do so until well after the start of the year, but I have strong reason to believe this broke our app very hard on 3.24. Specifically, when running 3.24.0-beta.2, effectively every single test in the suite either completely fails to run or dies on instantiation of link-to's, with errors like:
When I say it hits nearly all our tests: out of ~30,000 tests, only ~6,900 run, and of those somewhere around 6,000–6,100 fail with this error. 😬 |
|
@chriskrycho - ya, it requires changes in ember-engines to accomodate (otherwise we double prefix) |
|
@chriskrycho - Can you create an issue on ember-engines repo and cross link here (so we can track fixing)? |
|
yes, we need to make changes to LinkComponent. Also it requires changes to Ember Engines Guides. |
|
I filed a bug there; note also that we should make a point to call out the need to upgrade engines when upgrading to 3.24 in the blog post. |
See ember-engines/ember-engines#692 (comment). This should allow ember-engines to remove the
<LinkTo>monkey patch (at least that's the intention).Not sure what versions to target (if any) for the backport. This could arguably be a breaking bugfix for people using engines but not using the ember-engines addon? I don't know if that combination exists in the real world though.