Skip to content
This repository was archived by the owner on Apr 4, 2025. It is now read-only.

Conversation

@gregmagolan
Copy link
Contributor

No description provided.

@gregmagolan gregmagolan requested a review from alexeagle June 25, 2018 18:17
@gregmagolan
Copy link
Contributor Author

Build & e2e working. Still have a karma test failure with a long call stack that I haven't looked into yet.

@gregmagolan
Copy link
Contributor Author

Karma test failure resolved

@buildsize
Copy link

buildsize bot commented Jun 26, 2018

File name Previous Size New Size Change
bundle.min.js 424.89 KB 412.45 KB -12.43 KB (3%)

@gregmagolan
Copy link
Contributor Author

Hmmm. Looks like the added memory load of building angular is crashing the JVM again. Lowered the memory usage in bazelrc some more.

@gregmagolan
Copy link
Contributor Author

gregmagolan commented Jun 26, 2018

For reference: without the types-patch in the Angular repo package.json these are the paths that were needed for zone.js.d.ts to be loaded in modules/types.d.ts for the angular build directly and for building angular from source:

// works for @angular repo as external build
// <reference path="../../../external/angular_deps/node_modules/zone.js/dist/zone.js.d.ts" />

// works for @angular repo build
// <reference path="../external/angular_deps/node_modules/zone.js/dist/zone.js.d.ts" />

@angular angular deleted a comment Aug 1, 2018
@angular angular deleted a comment Aug 1, 2018
@angular angular deleted a comment Aug 1, 2018
@angular angular deleted a comment Aug 1, 2018
@angular angular deleted a comment Aug 1, 2018
@angular angular deleted a comment Aug 1, 2018
@angular angular deleted a comment Aug 1, 2018
@angular angular deleted a comment Aug 1, 2018
@angular angular deleted a comment Aug 1, 2018
@angular angular deleted a comment Aug 1, 2018
@angular angular deleted a comment Aug 1, 2018
@angular angular deleted a comment Aug 1, 2018
@angular angular deleted a comment Aug 1, 2018
@angular angular deleted a comment Aug 1, 2018
@angular angular deleted a comment Aug 1, 2018
@jorgeucano
Copy link

@gregmagolan this PR fix the ": Error: Could not resolve ./router.ngfactory " error ???

@angular angular deleted a comment Aug 6, 2018
@angular angular deleted a comment Aug 6, 2018
@angular angular deleted a comment Aug 6, 2018
@angular angular deleted a comment Aug 6, 2018
@angular angular deleted a comment Aug 9, 2018
@angular angular deleted a comment Aug 9, 2018
@angular angular deleted a comment Aug 9, 2018
@angular angular deleted a comment Aug 9, 2018
@angular angular deleted a comment Aug 9, 2018
@gregmagolan
Copy link
Contributor Author

@jorgeucano Yes. This PR resolves angular/angular#24521 which was introduced in Angular 6.0.5.

@angular angular deleted a comment Aug 9, 2018
@angular angular deleted a comment Aug 9, 2018
@angular angular deleted a comment Aug 9, 2018
@angular angular deleted a comment Aug 9, 2018
@angular angular deleted a comment Aug 9, 2018

# Install the dependencies from NPM
- run: bazel run @nodejs//:yarn install
# TODO(gmagolan): use `bazel run :install` once bootstrap issue resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

file an issue to reference here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

which repo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actual = "@nodejs//:yarn",
)

alias(
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe a comment that this is the place ts_library will look by default, allows us to omit explicit tsconfig attributes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

WORKSPACE Outdated
node_repositories(package_json = ["//:package.json"])

# 0.11.3: proper module resolution & check_rules_nodejs_version
check_rules_nodejs_version("0.11.3")
Copy link
Contributor

Choose a reason for hiding this comment

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

yeah users shouldn't need to do transitive checks

tools/bazel.rc Outdated
test --test_output=errors

################################
# Temporary Settings for Ivy #
Copy link
Contributor

Choose a reason for hiding this comment

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

I mean, change the comment, ppl will think this means we are enabling Ivy in legacy mode

@@ -0,0 +1,11 @@
const protractorUtils = require('@angular/bazel/protractor-utils');
Copy link
Contributor

Choose a reason for hiding this comment

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

since this repo is the canonical example, could you add more explanatory comments about when this file executes and how it relates to the protractor rule and the code under test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

srcs = glob(["*.ts"]),
)

protractor_web_test_suite(
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we should add comments with links to API docs now that they are published

Copy link
Contributor Author

Choose a reason for hiding this comment

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

where did those get published to?

@@ -1,3 +1,5 @@
import 'tslib';
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe something in Angular should depend on it. Who actually uses a symbol that requires tslib to be present? It's related to the --noHelpers flag in the tsconfig right?

@angular angular deleted a comment Aug 9, 2018
@angular angular deleted a comment Aug 9, 2018
@angular angular deleted a comment Aug 9, 2018
@angular angular deleted a comment Aug 9, 2018
@angular angular deleted a comment Aug 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants