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

Fix a problem where plain functions can't be used as helper if they are not passed through the strict mode context object #1389

Conversation

Windvis
Copy link
Contributor

@Windvis Windvis commented Apr 9, 2022

While working on emberjs/ember.js#20052 I noticed that an error was thrown when using a function that was stored on a component class property. The hasInternalHelperManager util is used there but it doesn't pass through the default helper manager fallback behavior code so it returns false and the error is shown.

It seems functions passed as strict mode context values follow a different code path than component arguments and class properties so the error wasn't shown in the tests of the original PR.

@Windvis
Copy link
Contributor Author

Windvis commented Apr 9, 2022

I'm not sure what the performance check faillure is about. 🤔

@chancancode
Copy link
Contributor

Thanks for catching this!

@Windvis Windvis force-pushed the fix/default-helper-manager-functions-as-props-and-args branch from ce29ff1 to aca1a5c Compare April 13, 2022 07:57
@chancancode
Copy link
Contributor

The perf test also passed so I think it was just a random failure last time. I'll check back tomorrow to give you some time to make the change you wanted. If not I will probably merge it anyway since it can always be refactored later.

…re not passed through the strict mode context object

It seems functions passed as strict mode context values follow a different code path than component arguments and class properties so the error wasn't shown there.
@Windvis Windvis force-pushed the fix/default-helper-manager-functions-as-props-and-args branch from aca1a5c to ebdc58d Compare April 13, 2022 08:35
@chancancode chancancode merged commit f8c9e21 into glimmerjs:master Apr 13, 2022
@Windvis Windvis deleted the fix/default-helper-manager-functions-as-props-and-args branch April 13, 2022 08:45
@Windvis
Copy link
Contributor Author

Windvis commented Apr 13, 2022

@chancancode Thank you for the quick reviewing!

@chancancode
Copy link
Contributor

@Windvis published v0.84.2 with this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants