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

[Bug] Angular @T Decorator #187

Open
rgant opened this issue Apr 15, 2023 · 2 comments
Open

[Bug] Angular @T Decorator #187

rgant opened this issue Apr 15, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@rgant
Copy link
Contributor

rgant commented Apr 15, 2023

Current Behavior
The Angular @T decorator does not function as expected with an instanceConfig parameter because that results in an asynchronous getter. Which is not valid JavaScript.

Expected Behavior
The addInstance call is a Promise.

Promises are not valid inside of getters.

Since the addInstance promise is not awaited, the call to translationService.translate with the instanceConfig.alias is not going to use the instance added since the translate call happens in synchronous code, but the addInstance call is asynchronous.

Additionally the test for this code path is flawed and does not produce an actual test.

Using a setTimeout (asynchronous code) inside a synchronous test does not function correctly. Because the current ng test configuration is setup to run only a single time the setTimeout with 1000 millisecond delay is silently dropped to no effect. You can confirm this by observing that the test execution time of ng test is less than one second execution time:

Chrome Headless 112.0.5615.49 (Mac OS 10.15.7): Executed 57 of 57 SUCCESS (0.185 secs / 0.155 secs)

Steps to Reproduce
Changing the expectation inside the setTimeout to one that should fail does not result in any failures during the test run.

Possible Solution
Require that instances be added before the decorator is used. This is probably difficult to actually achieve in practice.

Possible Implementation
I will be removing the asynchronous parts of this feature in #186 in a future push to this branch.

@rgant rgant added the bug Something isn't working label Apr 15, 2023
@rgant rgant changed the title [Bug] @T Decorator [Bug] Angular @T Decorator Apr 15, 2023
@pablotransifex
Copy link
Contributor

pablotransifex commented Apr 15, 2023

@rgant Sounds legit, I probably missed that async limitation in getters. I'll try to review the Angular 14 related changes this upcoming week, and let you know.

@pablotransifex
Copy link
Contributor

@rgant I will add the changes you had in you PR related to the decorator as soon as I can to fix this issue. The Angular 14 upgrade PR has been merged, I prefer to keep this one separated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants