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

Ivy hello world with devserver #22788

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions WORKSPACE
Expand Up @@ -2,9 +2,9 @@ workspace(name = "angular")

http_archive(
name = "build_bazel_rules_nodejs",
url = "https://github.com/bazelbuild/rules_nodejs/archive/0.5.3.zip",
strip_prefix = "rules_nodejs-0.5.3",
sha256 = "17a5515f59777b00cb25dbc710017a14273f825029b2ec60e0969d28914870be",
url = "https://github.com/bazelbuild/rules_nodejs/archive/25bb70fb67bddcc257b869f434ccc0fd130ec3bd.zip",
strip_prefix = "rules_nodejs-25bb70fb67bddcc257b869f434ccc0fd130ec3bd",
sha256 = "11c0d73bdcb4b2608abbe5967be5a910bdaebf848eb13e4e7f8413bbdeb940b8",
)

load("@build_bazel_rules_nodejs//:defs.bzl", "check_bazel_version", "node_repositories")
Expand Down
9 changes: 9 additions & 0 deletions packages/core/src/render3/assert.ts
Expand Up @@ -52,6 +52,15 @@ export function assertNotNull<T>(actual: T, msg: string) {
}
}

export function assertComponentType(
actual: any,
msg: string =
'Type passed in is not ComponentType, it does not have \'ngComponentDef\' property.') {
if (!actual.ngComponentDef) {
throwError(msg);
}
}

function throwError(msg: string): never {
throw new Error(`ASSERTION ERROR: ${msg}`);
}
13 changes: 9 additions & 4 deletions packages/core/src/render3/component.ts
Expand Up @@ -8,10 +8,11 @@

// We are temporarily importing the existing viewEngine from core so we can be sure we are
// correctly implementing its interfaces for backwards compatibility.
import {Type} from '../core';
import {Injector} from '../di/injector';
import {ComponentRef as viewEngine_ComponentRef} from '../linker/component_factory';

import {assertNotNull} from './assert';
import {assertComponentType, assertNotNull} from './assert';
import {queueInitHooks, queueLifecycleHooks} from './hooks';
import {CLEAN_PROMISE, _getComponentHostLElementNode, baseDirectiveCreate, createLView, createTView, enterView, getRootView, hostElement, initChangeDetectorIfExisting, locateHostElement, renderComponentOrTemplate} from './instructions';
import {ComponentDef, ComponentType} from './interfaces/definition';
Expand Down Expand Up @@ -113,9 +114,13 @@ export const NULL_INJECTOR: Injector = {
* @param options Optional parameters which control bootstrapping
*/
export function renderComponent<T>(
componentType: ComponentType<T>, opts: CreateComponentOptions = {}): T {
componentType: ComponentType<T>|
Type<T>/* Type as workaround for: Microsoft/TypeScript/issues/4881 */
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be a TODO...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don' think so. These are all over the code base, and there will have to be big clean up once that is resolved.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should ComponentType be an union type with a TODO then ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, because then it would apply to all cases and we would lose type safety internally. We really just want it on the user code Ivy boundary.

Copy link
Contributor

Choose a reason for hiding this comment

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

Like a FromUserComponentType ?

,
opts: CreateComponentOptions = {}): T {
ngDevMode && assertComponentType(componentType);
const rendererFactory = opts.rendererFactory || domRendererFactory3;
const componentDef = componentType.ngComponentDef as ComponentDef<T>;
const componentDef = (componentType as ComponentType<T>).ngComponentDef as ComponentDef<T>;
if (componentDef.type != componentType) componentDef.type = componentType;
let component: T;
const hostNode = locateHostElement(rendererFactory, opts.host || componentDef.tag);
Expand All @@ -135,7 +140,7 @@ export function renderComponent<T>(
try {
// Create element node at index 0 in data array
elementNode = hostElement(hostNode, componentDef);
// Create directive instance with n() and store at index 1 in data array (el is 0)
// Create directive instance with factory() and store at index 1 in data array (el is 0)
component = rootContext.component =
baseDirectiveCreate(1, componentDef.factory(), componentDef) as T;
initChangeDetectorIfExisting(elementNode.nodeInjector, component);
Expand Down
15 changes: 13 additions & 2 deletions packages/core/test/bundling/hello_world/BUILD.bazel
@@ -1,11 +1,12 @@
package(default_visibility = ["//visibility:public"])

load("//tools:defaults.bzl", "ts_library")
load("//tools:defaults.bzl", "ts_library", "ivy_ng_module")
load("//tools/symbol-extractor:index.bzl", "js_expected_symbol_test")
load("//packages/bazel/src:ng_rollup_bundle.bzl", "ng_rollup_bundle")
load("@build_bazel_rules_nodejs//:defs.bzl", "jasmine_node_test")
load("@build_bazel_rules_typescript//:defs.bzl", "ts_devserver")

ts_library(
ivy_ng_module(
name = "hello_world",
srcs = ["index.ts"],
deps = [
Expand Down Expand Up @@ -54,3 +55,13 @@ js_expected_symbol_test(
src = ":bundle.min_debug.js",
golden = ":bundle.golden_symbols.json",
)

ts_devserver(
name = "devserver",
static_files = [
":bundle.min_debug.js",
":bundle.min.js",
"index.html",
],
deps = [],
)
Expand Up @@ -8,6 +8,12 @@
{
"name": "EMPTY_RENDERER_TYPE_ID"
},
{
"name": "HelloWorld"
},
{
"name": "INeedToExistEvenThoughtIAmNotNeeded"
},
{
"name": "NG_HOST_SYMBOL"
},
Expand Down Expand Up @@ -68,6 +74,9 @@
{
"name": "defineComponent"
},
{
"name": "defineInjector"
},
{
"name": "detectChangesInternal"
},
Expand Down
31 changes: 31 additions & 0 deletions packages/core/test/bundling/hello_world/index.html
@@ -0,0 +1,31 @@
<!doctype html>

<html>
<head>
<title>Angular Hello World Example</title>
</head>
<body>
<!-- The Angular application will be bootstrapped into this element. -->
<hello-world></hello-world>

<!--
Script tag which bootstraps the application. Use `?debug` in URL to select
the debug version of the script.

There are two scripts sources: `bundle.min.js` and `bundle.min_debug.js` You can
switch between which bundle the browser loads to experiment with the application.

- `bundle.min.js`: Is what the site would serve to their users. It has gone
through rollup, build-optimizer, and uglify with tree shaking.
- `bundle.min_debug.js`: Is what the developer would like to see when debugging
the application. It has also done through full pipeline of rollup, build-optimizer,
and uglify, however special flags were passed to uglify to prevent inlining and
property renaming.
-->
<script>
document.write('<script src="' +
(document.location.search.endsWith('debug') ? '/bundle.min_debug.js' : '/bundle.min.js') +
'"></' + 'script>');
</script>
</body>
</html>
21 changes: 9 additions & 12 deletions packages/core/test/bundling/hello_world/index.ts
Expand Up @@ -6,19 +6,16 @@
* found in the LICENSE file at https://angular.io/license
*/

import {ɵT as T, ɵdefineComponent as defineComponent, ɵrenderComponent as renderComponent} from '@angular/core';
import {Component, NgModule, ɵrenderComponent as renderComponent} from '@angular/core';

class HelloWorld {
static ngComponentDef = defineComponent({
type: HelloWorld,
tag: 'hello-world',
factory: () => new HelloWorld(),
template: function HelloWorldTemplate(ctx: HelloWorld, cm: boolean) {
if (cm) {
T(0, 'Hello World!');
}
}
});
@Component({selector: 'hello-world', template: 'Hello World!'})
export class HelloWorld {
}
// TODO(misko): Forgetting to export HelloWorld and not having NgModule fails silently.

@NgModule({declarations: [HelloWorld]})
export class INeedToExistEvenThoughtIAmNotNeeded {
}
// TODO(misko): Package should not be required to make this work.

renderComponent(HelloWorld);
28 changes: 23 additions & 5 deletions tools/symbol-extractor/symbol_extractor.ts
Expand Up @@ -22,15 +22,17 @@ export class SymbolExtractor {
static parse(path: string, contents: string): Symbol[] {
const symbols: Symbol[] = [];
const source: ts.SourceFile = ts.createSourceFile(path, contents, ts.ScriptTarget.Latest, true);
let fnDepth = 0;
let fnRecurseDepth = 0;
function visitor(child: ts.Node) {
// Left for easier debugging.
// console.log('>>>', ts.SyntaxKind[child.kind]);
switch (child.kind) {
case ts.SyntaxKind.FunctionExpression:
fnDepth++;
if (fnDepth <= 1) {
// Only go into function expression once for the outer closure.
fnRecurseDepth++;
Copy link
Contributor

Choose a reason for hiding this comment

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

don't you ever need to -- ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, added.

Copy link
Contributor

Choose a reason for hiding this comment

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

do we need the test to be updated ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For this to fail, we would have to have more than one top level closure, which does not happen in real life. Not sure that would be useful, what do you think.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would update the test but it's up to you.

if (fnRecurseDepth <= 1) {
ts.forEachChild(child, visitor);
}
fnRecurseDepth--;
break;
case ts.SyntaxKind.SourceFile:
case ts.SyntaxKind.VariableStatement:
Expand All @@ -44,9 +46,13 @@ export class SymbolExtractor {
break;
case ts.SyntaxKind.VariableDeclaration:
const varDecl = child as ts.VariableDeclaration;
if (varDecl.initializer) {
if (varDecl.initializer && fnRecurseDepth !== 0) {
symbols.push({name: varDecl.name.getText()});
}
if (fnRecurseDepth == 0 &&
isRollupExportSymbol(child.parent as ts.VariableDeclarationList)) {
ts.forEachChild(child, visitor);
}
break;
case ts.SyntaxKind.FunctionDeclaration:
const funcDecl = child as ts.FunctionDeclaration;
Expand Down Expand Up @@ -114,3 +120,15 @@ function toSymbol(v: string | Symbol): Symbol {
function toName(symbol: Symbol): string {
return symbol.name;
}

/**
* Detects if VariableDeclarationList is format `var x = function(){}()`;
*
* Rollup produces this format when it wants to export symbols from a bundle.
* @param child
*/
function isRollupExportSymbol(child: ts.VariableDeclarationList): boolean {
if (child.declarations.length !== 1) return false;
const decl: ts.VariableDeclaration = child.declarations[0];
return !!(decl.initializer && decl.initializer.kind == ts.SyntaxKind.CallExpression);
}
26 changes: 26 additions & 0 deletions tools/symbol-extractor/symbol_extractor_spec/iife_with_export.js
@@ -0,0 +1,26 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

/**
* Rollup exports symbols in this particular way. This test demonstrates that we can correctly read
* symbols.
*/
var fooBar = function(exports) {
'use strict';
// tslint:disable-next-line:no-console
console.log('Hello, Alice in Wonderland');
var A = function() {
function A() {}
A.prototype.a = function() { return document.a; };
return A;
}();
// tslint:disable-next-line:no-console
console.error(new A().a());
exports.A = A;
return exports;
}({});
@@ -0,0 +1,3 @@
[
"A"
]