[Bugfix release] allow class-based helpers to work in strict-mode #19878
Conversation
| module('tests/integration/components/gjs', function (hooks) { | ||
| setupRenderingTest(hooks); | ||
|
|
||
| test('it works with ember helpers', async function (assert) { |
There was a problem hiding this comment.
I added a whole app for just this tset because I couldn't find any other place where I'd have access to all the thing that this test requires.
If such a place exists, I'm happy to move this test there.
I think having a full app though to make sure things are roughly still wired up ok is good
| @@ -179,7 +179,7 @@ class ClassicHelperManager implements HelperManager<ClassicHelperStateBucket> { | |||
| } | |||
|
|
|||
| getDebugName(definition: ClassHelperFactory) { | |||
| return getDebugName!(definition.class!['prototype']); | |||
| return getDebugName!((definition.class || definition)!['prototype']); | |||
There was a problem hiding this comment.
when a class-based helper is used in strict mode, there is no class property
|
might this be related? #19518 |
|
@patricklx I think it might be!! yeah! |
|
This fix is probably ok, but I would like it tested in the existing test harness and not an entirely new suite. Adding a whole new kind of smoke test should a) be decoupled into its own PR and b) does not preclude ensuring regressions have coverage added in the internal non-smoke test suite. That is the main blocker I see here. Is this a bugfix-release? Bugfix-lts? |
It's a bug on 4.0+, afaict (I haven't checked earlier) -- so, bugfix-release?
where would a test like this go?
can do! |
NullVoxPopuli
changed the title
NullVoxPopuli
force-pushed
the
smoke-test-and-repro-of-19877
branch
from
103d0ed
to
a20c870
Compare
|
I'm having issues with the test. any help is much appreciated <3 |
NullVoxPopuli
force-pushed
the
smoke-test-and-repro-of-19877
branch
from
7bd8709
to
6801d00
Compare
NullVoxPopuli
force-pushed
the
smoke-test-and-repro-of-19877
branch
from
6801d00
to
6bf0153
Compare
|
Merging on approvals from #19884 |
I thought we could take first-class helper support for granted since it was supposed to be introduced before 3.28 (our oldest supported ember), but due to: emberjs/ember.js#19878 we can't rely on it until ember 4.2.
Resolves #19877