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

Examples angular does not work on legacy browsers #1869

Closed
kyubisation opened this issue May 5, 2020 · 16 comments
Closed

Examples angular does not work on legacy browsers #1869

kyubisation opened this issue May 5, 2020 · 16 comments
Labels
Can Close? We will close this in 30 days if there is no further activity

Comments

@kyubisation
Copy link
Contributor

🐞 bug report

Affected Rule

https://github.com/bazelbuild/rules_nodejs/tree/master/examples/angular
yarn serve-prod

Is this a regression?

Partially

Description

The Angular example in https://github.com/bazelbuild/rules_nodejs/tree/master/examples/angular does not work at all.

Problems:

  • There seem to be some search and replace errors (e.g. load("@npm//@bazel/typescript:index.bzl", "ts_library") should probably be load("@npm_bazel_typescript//:index.bzl", "ts_library") and others)
  • The es5 bundle for legacy browsers (like IE11) does not work, due to babel bundling to require( imports and system.js not providing require imports
  • The es2015 bundle does not work for (legacy) Edge and simply outputs SCRIPT5022: SCRIPT5022: Syntax error
  • Optional: The docker rules do not work on Windows and break on installation (This I can work around by simply removing the relevant rules)
  • Optional: On Windows running yarn serve-prod (or bazel run //src:prodserver) can result in an error No 'bazel-out' folder found in path 'c:/users/lukas/_bazel~1/zhhytyhl/execroot/exampl~1/bazel-~1/x64_wi~1/bin/src/prodse~2.run'! (which I can work around by changing directory to the folder of prodserver.bat and starting it from there). This happens due to looking for /bazel-root/ in resolveRoot in the linker (https://github.com/bazelbuild/rules_nodejs/blob/master/internal/linker/link_node_modules.ts#L124) and process.cwd() only provides the shortened path on unclear circumstances. As far as I can tell, there is no easy way to convert the shortened path in nodejs.

🔬 Minimal Reproduction

https://github.com/bazelbuild/rules_nodejs/tree/master/examples/angular
yarn serve-prod

Anything else relevant?

First; I'm very grateful for the NodeJS rules for Bazel.
However I can't help but feel a bit disheartened, when the official example does not work. Especially with the deprecation of ng_module.

Maybe for comparison; Our current implementation (which also does not work due to the above mentioned problems): https://github.com/sbb-design-systems/sbb-angular/blob/master/src/showcase/BUILD.bazel

@gregmagolan
Copy link
Collaborator

gregmagolan commented May 5, 2020

Its expected that that examples won't work when run from within the nested repository on any given commit; especially on the current master where we're churning on breaking changes such as the change to load("@npm//@bazel/typescript:index.bzl", "ts_library") from load("@npm_bazel_typescript//:index.bzl", "ts_library").

If you checkout the repo at a release then you'll have better luck with the examples. The latest working stand-alone examples/angular is at the 7ac0ada commit (https://github.com/bazelbuild/rules_nodejs/tree/7ac0ada272c9a234c1eed6c8ba9d1e3f2e1ac4b3/examples/angular). This is the chore: update lock files for release commit after the 1.6.1 release on the 1.x branch. This should be better documented :)

The reason they don't work on an given commit from within the nested workspace is that examples are kept up to date with the root workspace but their WORKSPACE files and package.json files still reference the last release (which is currently 1.6). When running stand-alone there will be version skew between the examples code (which is mean to work with the rules in the root workspace) and the last release of rules_nodejs which they pull in. We test them in the repo with a bazel_integration_test rule that modifies their WORKSPACES files and package.json references with generated packages that are build in the root WORKSPACE. For example, you can run examples/angular tests on master with bazel test //examples:examples_angular.

At the very least I think we should document this in the main README since users will naturally gravitate towards checking out the latest and looking at the examples. @alexeagle thoughts?

@gregmagolan
Copy link
Collaborator

Windows support is general problem in the Bazel and in the Bazel rules ecosystem and not something unique to rules_nodejs. We're one of the few rule sets that has CI against Windows but not everything works and a lot of that is beyond our control. We have quite a few fix-windows tags scattered around in the repo.

@gregmagolan
Copy link
Collaborator

gregmagolan commented May 5, 2020

With ts_library es5 & es2015 outputs, you may have better luck with the architect() rules used in examples/angular_bazel_architect. That example needs polish but it uses the angular-cli architect instead of ts_library which will produce the same outputs that the angular-cli would produce. The UX with that approach will be better than with using ts_library opinionated module format outputs that align with google3. With architect(), if it works in the angular-cli it should work with the architect() rule. There is, however, still a lot of work to be done to make that polish that example including adding a devserver and limited bandwidth to work on it.

@kyubisation
Copy link
Contributor Author

Thank you very much for your answer. I will check if I can use the architect rule with our small monorepo.
I apologize for the frustrated issue description.

@kyubisation
Copy link
Contributor Author

@gregmagolan Maybe an additional comment; The current implementation for es5 in the angular example cannot work, so I'm not sure what kind of purpose it serves?

@kyubisation
Copy link
Contributor Author

As an update; examples/angular_bazel_architect at tag 1.6.1 also does not work. It errors with an unhelpful message.

PS D:\Projects\rules_nodejs\examples\angular_bazel_architect> bazel build //:build --verbose_failures --define=VERBOSE_LOG=1
INFO: Invocation ID: 8af56210-c8f7-47a3-8c22-cae9824c7ecf
INFO: Build option --define has changed, discarding analysis cache.
INFO: Analyzed target //:build (0 packages loaded, 72069 targets configured).
INFO: Found 1 target...
ERROR: D:/projects/rules_nodejs/examples/angular_bazel_architect/BUILD.bazel:8:1: Action build failed (Exit 1): architect.bat failed: error executing command
  cd C:/users/lukas/_bazel_lukas/f6ull4h5/execroot/angular_bazel_architect
  SET BAZEL_NODE_MODULES_ROOT=npm/node_modules
    SET COMPILATION_MODE=fastbuild
    SET NG_BUILD_CACHE=false
  bazel-out/host/bin/external/npm/@angular-devkit/architect-cli/bin/architect.bat frontend:build --outputPath=bazel-out/x64_windows-fastbuild/bin/build --bazel_node_modules_manifest=bazel-out/x64_windows-fastbuild/bin/_build.module_mappings.json --nobazel_patch_module_resolver
Execution platform: @local_config_platform//:host

events.js:187
      throw er; // Unhandled 'error' event
      ^

Error: spawn C:\users\lukas\_bazel_lukas\f6ull4h5\external\build_bazel_rules_nodejs\internal\node\_node_bin\node ENOENT
    at Process.ChildProcess._handle.onexit (internal/child_process.js:264:19)
    at onErrorNT (internal/child_process.js:456:16)
    at processTicksAndRejections (internal/process/task_queues.js:80:21)
Emitted 'error' event on ChildProcess instance at:
    at Process.ChildProcess._handle.onexit (internal/child_process.js:270:12)
    at onErrorNT (internal/child_process.js:456:16)
    at processTicksAndRejections (internal/process/task_queues.js:80:21) {
  errno: 'ENOENT',
  code: 'ENOENT',
  syscall: 'spawn C:\\users\\lukas\\_bazel_lukas\\f6ull4h5\\external\\build_bazel_rules_nodejs\\internal\\node\\_node_bin\\node',
  path: 'C:\\users\\lukas\\_bazel_lukas\\f6ull4h5\\external\\build_bazel_rules_nodejs\\internal\\node\\_node_bin\\node',
  spawnargs: [
    '--require',
    'C:/users/lukas/_bazel_lukas/f6ull4h5/external/build_bazel_rules_nodejs/internal/node/node_patches.js',
    '--preserve-symlinks',
    '--preserve-symlinks-main',
    'C:\\users\\lukas\\_bazel_lukas\\f6ull4h5\\execroot\\angular_bazel_architect\\node_modules\\jest-worker\\build\\workers\\processChild.js'
  ]
}
Target //:build failed to build
INFO: Elapsed time: 16.208s, Critical Path: 11.80s
INFO: 0 processes.
FAILED: Build did NOT complete successfully

@kyubisation kyubisation reopened this May 6, 2020
@kyubisation
Copy link
Contributor Author

I have made some progress with creating a initializer script, which loads either the es5 or the es2015 depending on circumstances.
Would you like a pull request for the angular example after I have finalized it?

@alexeagle
Copy link
Collaborator

An "initializer script" is probably the wrong thing, we generally key the modern/legacy JS based on the availability of <script type="module"> like this https://github.com/bazelbuild/rules_nodejs/blob/master/examples/webapp/index.html#L8-L9

I'm still not sure if there is something we need to do here. Is there a bug in rules_nodejs or the example?

@kyubisation
Copy link
Contributor Author

The docs state that the example is now published to https://bazel.angular.io/example/, which I appreciate. However it does not seem to work yet? At least the output files in https://bazel.angular.io/example/ do not correspond to the output files when I build src:prodapp locally.

The following problems persist and I would argue that this is a bug in the example:

  • The es5 bundle for legacy browsers (like IE11) does not work, due to babel bundling to require( imports and system.js not providing require imports
    This seems to be partially solved on https://bazel.angular.io/example/, when looking at https://bazel.angular.io/bundle.min.js?v=1568318376226 (which fails to load main.prod.js with a 404). There is however no corresponding code in the example.
  • The es2015 bundle does not work for (legacy) Edge and simply outputs SCRIPT5022: SCRIPT5022: Syntax error
    We worked around this with the mentioned initializer script when we noticed it could handle the es5 bundle: https://github.com/sbb-design-systems/sbb-angular/blob/master/src/showcase/index.prod.html#L105-L125

An additional question; Why do you not fingerprint the index.js files (https://github.com/bazelbuild/rules_nodejs/blob/master/examples/angular/src/example/index.prod.html#L20-L21) with a simple genrule?

@alexeagle
Copy link
Collaborator

Thanks for pointing out problems with legacy browsers and showing your workaround.

The bazel.angular.io site is owned by the Angular team and I expect they'll turn it down. We should find a new way to publish all our example apps to some static hosting. @mattem and I are looking at re-deploying our docsite so maybe it's similar enough? but probably we need an interested contributor to figure this out.

Fingerprinting sounds great, I think our example is just not comprehensive and shows the things we thought of. Would love to add that if you have time.

@alexeagle alexeagle changed the title Examples angular does not work Examples angular does not work on legacy browsers Aug 17, 2020
@kyubisation
Copy link
Contributor Author

I finally managed to invest a bit of time into WSL2, which should help me with running the examples.
While I am interested in improving the examples, I cannot yet promise a time frame.
For future reference; Is there anything you are strictly opposed to (e.g. initializer script, special handling of legacy Edge or custom genrules)?

@alexeagle
Copy link
Collaborator

I don't really care what's in the example, so long as it's easy for us to maintain and represents some kind of consensus from Angular developers about what's a good practice. If you want to add something that Angular CLI doesn't do, that's great because it shows how Bazel lets you customize the toolchain.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs in two weeks. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs!

@github-actions github-actions bot added the Can Close? We will close this in 30 days if there is no further activity label Oct 17, 2020
@github-actions
Copy link

github-actions bot commented Nov 1, 2020

This issue was automatically closed because it went two weeks without a reply since it was labeled "Can Close?"

@sss-first-user
Copy link

ng new is not working but generating below error
"legacyBrowsers is not defined"
legacyBrowsers is not defined

@PRASHANTPAL283
Copy link

PRASHANTPAL283 commented Jan 29, 2024

ng new is not working but generating below error
"legacyBrowsers is not defined"
legacyBrowsers is not defined
Can anyone please help to resolve this issue?
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Can Close? We will close this in 30 days if there is no further activity
Projects
None yet
Development

No branches or pull requests

5 participants