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 broken Angular dashboards in need of localization module #4937

Merged
merged 3 commits into from
May 5, 2021

Conversation

psybuzz
Copy link
Contributor

@psybuzz psybuzz commented May 5, 2021

Angular component-based dashboards currently fail to render. The
JS error at runtime points to the lack of an import of Angular's new
"localize" module, required for applications using Ivy and Angular 9+.
See https://angular.io/guide/migration-localize for details.

Note that the //tensorboard:dev target is fine. For some reason,
only the "prod" target //tensorboard is affected.

image

Manually tested by running bazel run tensorboard -- --logdir logs --bind_all, opening the 'Time Series' dashboard, and seeing the panel appear.

Googlers, see test sync cl/372052576

@google-cla google-cla bot added the cla: yes label May 5, 2021
@psybuzz psybuzz changed the title Fix broken Angular dashboards in need of localization symbol Fix broken Angular dashboards in need of localization module May 5, 2021
@psybuzz psybuzz marked this pull request as ready for review May 5, 2021 04:17
@psybuzz psybuzz requested a review from stephanwlee May 5, 2021 04:17
// Angular 9+ using Ivy apps that potentially do i18n, even transitively, must
// import this module, which adds a global symbol at runtime.
// https://angular.io/guide/migration-localize
import '@angular/localize/init'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move this to main_prod.ts. It is already included in the main_dev.ts so feel free to refactor and move them to bootstrap.ts.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@psybuzz psybuzz merged commit 069c744 into tensorflow:master May 5, 2021
@psybuzz
Copy link
Contributor Author

psybuzz commented May 5, 2021

Thanks!

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.

2 participants