-
Notifications
You must be signed in to change notification settings - Fork 6.8k
build: serve dev-app with bazel #16829
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
Conversation
5654b85
to
58f5bc3
Compare
* Serves the dev-app with Bazel using `ts_devserver` (windows compatible since bazelbuild/rules_typescript@4155687) * Serves dev-app with AOT mode. * ibazel can be used for watch-mode and live-reloading Currently due to limitations within the `ts_devserver` rule, livereloading with ibazel is not working. Manual reloading still works.
58f5bc3
to
c956bb9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM, just one other question that I could go either way on.
# Job that builds the dev-app with AOT. In order to speed up this job, the release output | ||
# from the workspace storage will be attached to this job. | ||
# ----------------------------------------------------------------------------------------- | ||
build_devapp_aot: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we still build the devapp as a test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be built as part of the bazel build job.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
"test": "bazel test //src/... --test_tag_filters=-e2e,-browser:firefox-local --build_tag_filters=-browser:firefox-local --build_tests_only", | ||
"test-firefox": "bazel test //src/... --test_tag_filters=-e2e,-browser:chromium-local --build_tag_filters=-browser:chromium-local --build_tests_only", | ||
"lint": "gulp lint && yarn -s bazel:format-lint", | ||
"e2e": "bazel test //src/... --test_tag_filters=e2e", | ||
"deploy": "gulp deploy:devapp", | ||
"deploy": "echo 'Not supported yet. Tracked with COMP-230'", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved this to the queue since I want to keep this deploy functional; we'll likely want to do some amount of IE and mobile testing with the MDC components as we progress
We're going to hold off on merging this until follow-up work to speed up the dev-app rebuild by making the targets more granular |
@jelbourn I'll re-add merge ready as the other PR is approved and ready. Feel free to remove if you think something is still missing. |
* Serves the dev-app with Bazel using `ts_devserver` (windows compatible since bazelbuild/rules_typescript@4155687) * Serves dev-app with AOT mode. * ibazel can be used for watch-mode and live-reloading Currently due to limitations within the `ts_devserver` rule, livereloading with ibazel is not working. Manual reloading still works. (cherry picked from commit 1162048)
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
ts_devserver
(windowscompatible since
bazelbuild/rules_typescript@4155687)
Currently due to limitations within the
ts_devserver
rule, livereloading with ibazel is notworking. Manual reloading still works. Created bazel-contrib/rules_nodejs#1036 for tracking.
Additionally rebuilding seems at first glance slower as with the gulp setup. This is because the
dev-app
has the same problem as thematerial-examples
and ideally it would have a bazel package file per module (modules are already available due to lazy loading)