Standalone core implementation by alxhub · Pull Request #45687 · angular/angular

alxhub · 2022-04-19T23:40:21Z

This PR is based on #45672, and adds JIT compilation as well as the actual public API for standalone components 🎉

There's still work to be done after this:

bootstrap API
Router lazy-loading updates
TestBed validation
documentation
lots more testing
etc.

packages/core/src/render3/jit/pipe.ts

pkozlowski-opensource · 2022-04-20T13:42:39Z

packages/core/src/metadata/directives.ts

   */
  jit?: true;
+
+  standalone?: boolean;


needs APIs docs

pkozlowski-opensource · 2022-04-20T13:42:48Z

packages/core/src/metadata/directives.ts

   */
  preserveWhitespaces?: boolean;
+
+  standalone?: boolean;


needs APIs docs

pkozlowski-opensource · 2022-04-20T13:42:54Z

packages/core/src/metadata/directives.ts

+  standalone?: boolean;
+
+  // TODO: better type here!
+  imports?: (Type<any>|any[])[];


needs APIs docs

pkozlowski-opensource · 2022-04-20T13:43:00Z

packages/core/src/metadata/directives.ts

   */
  pure?: boolean;
+
+  standalone?: boolean;


needs APIs docs

pkozlowski-opensource · 2022-04-20T13:43:39Z

packages/core/src/render3/jit/directive.ts

+    const dep = resolveForwardRef(rawDep);
+    if (!dep) {
+      // TODO: real error
+      throw new Error(`ForwardRef issue?`);


needs proper error

pkozlowski-opensource · 2022-04-20T13:43:50Z

packages/core/src/render3/jit/directive.ts

+    if (anyDef) {
+      if (!anyDef.standalone) {
+        // TODO: real error
+        throw new Error(`What are you doing? You imported a non-standalone thing!`);


needs proper error

pkozlowski-opensource · 2022-04-20T13:44:14Z

packages/core/src/render3/jit/directive.ts

    viewQueries: extractQueriesMetadata(type, propMetadata, isViewQuery),
    // TODO(alxhub): pass through the standalone flag from the directive metadata once standalone
    // functionality is fully rolled out.
-    isStandalone: false,


I guess the above comment is no longer relevant?

pkozlowski-opensource · 2022-04-20T13:44:34Z

packages/core/src/render3/jit/module.ts

+  }
+
+  // TODO: change the error message to be more user-facing and take standalone into account
+  throw new Error(`${type.name} does not have a module def (ɵmod property)`);


needs better error?

atscott · 2022-04-20T20:13:15Z

packages/core/src/render3/jit/directive.ts

+      // TODO: real error
+      throw new Error(`ForwardRef issue?`);
+    }
+    const ngModuleDef: NgModuleDef<any>|null = getNgModuleDef(dep);


Suggested change

const ngModuleDef: NgModuleDef<any>|null = getNgModuleDef(dep);

const ngModuleDef = getNgModuleDef<Type<unknown>>(dep);

Should this have a more restrictive type?

atscott · 2022-04-20T20:13:51Z

packages/core/src/render3/jit/directive.ts

+    if (ngModuleDef) {
+      dependencies.push({
+        kind: R3TemplateDependencyKind.NgModule,
+        type: dep as any,


Is the as any needed here?

atscott · 2022-04-20T20:22:08Z

packages/core/src/render3/jit/module.ts

    importingModule?: NgModuleType): void {
  if (verifiedNgModule.get(moduleType)) return;
+
+  // skip verifications of standalone components, direcrtives and pipes


nit: this comment is "what" but not why. It's mostly obvious that if (isStandalone(moduleType)) return; skips the verification. This comment should be updated to explain why or deleted. Also "direcrtives" is misspelled.

atscott · 2022-04-20T20:23:23Z

packages/core/src/render3/jit/module.ts

-  flatten(imports).map(unwrapModuleWithProvidersImports).forEach(mod => {
-    verifySemanticsOfNgModuleImport(mod, moduleType);
-    verifySemanticsOfNgModuleDef(mod, false, moduleType);
+  flatten(imports).map(unwrapModuleWithProvidersImports).forEach(modOrStandaloneCmpt => {


uber(!!) nit: We generally use Cmp for the shorthand of "component" if we're doing that. The "t" almost looks like a typo to me.

atscott · 2022-04-20T20:26:14Z

packages/core/src/render3/jit/module.ts

+  const ngModuleDef = getNgModuleDef(type);
+
+  // a standalone component, directive or pipe
+  if (ngModuleDef === null) {
+    return [type];
+  }


Suggested change

const ngModuleDef = getNgModuleDef(type);

// a standalone component, directive or pipe

if (ngModuleDef === null) {

return [type];

}

if (isStandalone(type)) {

return [type];

}

const ngModuleDef = getNgModuleDef(type, true);

nit: Personally I would think this would be easier to read and doesn't need a comment. But I can imagine it's slightly less optimal because there are a couple more lookups involved by calling isStandalone

Yep - exactly.

atscott · 2022-04-20T20:44:19Z

packages/core/src/render3/features/standalone_feature.ts

- * TODO(pk): documentation
+ * A feature that acts as a setup code for the {@see StandaloneService}.
+ *
+ * The most important responsaibility of this feature is to expose the "getStandaloneInjector"


nit: s/responsaibility/responsibility

AndrewKushnir

LGTM, just left a few comments. None of them are blockers and we can look into them in a followup PR.

packages/core/src/di/provider_collection.ts

AndrewKushnir · 2022-04-20T19:48:46Z

packages/core/src/render3/component_ref.ts

nit: can we add a TODO to add a comment here in a followup PR?

AndrewKushnir · 2022-04-20T19:50:46Z

packages/core/src/render3/definition.ts

nit: it'd be great to extract componentDefinition.standalone === true into a separate const and reuse in 2 places above (for code size optimization):

const standalone = componentDefinition.standalone === true;

(let's leave a TODO and I can cleanup in a followup PR)

packages/core/src/render3/interfaces/definition.ts

packages/core/src/render3/jit/directive.ts

packages/core/src/render3/jit/module.ts

AndrewKushnir · 2022-04-20T20:48:26Z

packages/core/src/render3/jit/module.ts

+  // skip verifications of standalone components, direcrtives and pipes
+  if (isStandalone(moduleType)) return;


Do we have a verification somewhere else or it should be implemented as a followup?

I'm not sure if "skip verification" here is a TODO for later - I think this means "skip NgModule verification"

Yeah, I was wondering if we do similar verification for imported standalone types somewhere else (thus we don't need it).

AndrewKushnir · 2022-04-20T20:51:23Z

packages/core/src/render3/jit/module.ts

-  const ngModuleDef = getNgModuleDef(type, true);
+  const ngModuleDef = getNgModuleDef(type);
+
+  // a standalone component, directive or pipe


nit: I think adding an assert here (in a followup PR) to check that a type is actually standalone would be great.

atscott

reviewed-for: size-tracking, public-api

alxhub · 2022-04-20T21:55:40Z

Caretaker: the functions here have already been reviewed for public-api (in a previous commit, they just didn't make it into the golden, for reasons). Consider it approved!

AndrewKushnir

Reviewed-for: size-tracking, public-api

This commit extracts the `importProvidersFrom` function and associated machinery into a separate file, as opposed to being colocated with `R3Injector`. Separating these functions will mitigate potential future circular dependencies as `importProvidersFrom` starts being used in different parts of the codebase.

This commit exposes the `standalone` flag on `@Directive`, `@Component`, and `@Pipe`, effectively making standalone components a part of Angular's public API. As part of this operation, it also implements JIT compilation for standalone types. Standalone types are Angular-decorated types which act as their own "declarations", where they would otherwise be declared in an NgModule. Marking an Angular type as standalone means that it can be used directly in other standalone components and in NgModules, without needing an associated NgModule to depend on it. In the case of a standalone component, template dependencies which would otherwise be specified by an NgModule are instead specified directly on the component itself, via the `imports` field. Other standalone types can be imported, as well as NgModules.

This commit refactors `importProvidersFrom` to support pulling providers from the dependencies of a standalone component, in addition to NgModules. Tests will be added in a future commit when standalone components can be created without calling private APIs.

This commit implements the `StandaloneFeature` which provides for the creation of standalone injectors, for those components which need them. The feature-based implementation ensures the machinery for standalone injectors is properly tree-shakable.

This commit reorganizes the tests around the EnvironmentInjector and its use for standalone injectors, and adds a number of new test cases.

This commit adds some internal documentation.

atscott · 2022-04-20T23:01:57Z

This PR was merged into the repository by commit 9e4c4bc.

…45687) This commit exposes the `standalone` flag on `@Directive`, `@Component`, and `@Pipe`, effectively making standalone components a part of Angular's public API. As part of this operation, it also implements JIT compilation for standalone types. Standalone types are Angular-decorated types which act as their own "declarations", where they would otherwise be declared in an NgModule. Marking an Angular type as standalone means that it can be used directly in other standalone components and in NgModules, without needing an associated NgModule to depend on it. In the case of a standalone component, template dependencies which would otherwise be specified by an NgModule are instead specified directly on the component itself, via the `imports` field. Other standalone types can be imported, as well as NgModules. PR Close #45687

…45687) This commit refactors `importProvidersFrom` to support pulling providers from the dependencies of a standalone component, in addition to NgModules. Tests will be added in a future commit when standalone components can be created without calling private APIs. PR Close #45687

This commit implements the `StandaloneFeature` which provides for the creation of standalone injectors, for those components which need them. The feature-based implementation ensures the machinery for standalone injectors is properly tree-shakable. PR Close #45687

This commit reorganizes the tests around the EnvironmentInjector and its use for standalone injectors, and adds a number of new test cases. PR Close #45687

This commit adds some internal documentation. PR Close #45687

This commit updates the standalone-related logic to address the feedback from previous code reviews (specifically: angular#45687 (comment)).

…45949) This commit updates the standalone-related logic to address the feedback from previous code reviews (specifically: #45687 (comment)). PR Close #45949

angular-automatic-lock-bot · 2022-06-19T16:06:06Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

pullapprove bot requested review from AndrewKushnir, atscott, dylhunn and jessicajaniuk April 19, 2022 23:40

josephperrott added the area: core Issues related to the framework runtime label Apr 19, 2022

ngbot bot added this to the Backlog milestone Apr 19, 2022

alxhub force-pushed the standalone branch 3 times, most recently from a0ddeba to 74b9350 Compare April 20, 2022 13:37

pkozlowski-opensource reviewed Apr 20, 2022

View reviewed changes

packages/core/src/render3/jit/pipe.ts Outdated Show resolved Hide resolved

pkozlowski-opensource reviewed Apr 20, 2022

View reviewed changes

packages/core/src/metadata/directives.ts

*/

jit?: true;

standalone?: boolean;

Copy link

Member

pkozlowski-opensource Apr 20, 2022

Choose a reason for hiding this comment

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

needs APIs docs

pkozlowski-opensource reviewed Apr 20, 2022

View reviewed changes

packages/core/src/metadata/directives.ts

*/

preserveWhitespaces?: boolean;

standalone?: boolean;

Copy link

Member

pkozlowski-opensource Apr 20, 2022

Choose a reason for hiding this comment

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

needs APIs docs

pkozlowski-opensource reviewed Apr 20, 2022

View reviewed changes

packages/core/src/metadata/directives.ts

*/

pure?: boolean;

standalone?: boolean;

Copy link

Member

pkozlowski-opensource Apr 20, 2022

Choose a reason for hiding this comment

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

needs APIs docs

pkozlowski-opensource reviewed Apr 20, 2022

View reviewed changes

pkozlowski-opensource force-pushed the standalone branch 4 times, most recently from 3eff87a to 1af65d9 Compare April 20, 2022 18:05

alxhub force-pushed the standalone branch from 1af65d9 to 85cc969 Compare April 20, 2022 19:41

alxhub added the target: minor This PR is targeted for the next minor release label Apr 20, 2022

alxhub force-pushed the standalone branch from 85cc969 to 94d2c8c Compare April 20, 2022 20:21

atscott reviewed Apr 20, 2022

View reviewed changes

pullapprove bot requested review from atscott April 20, 2022 20:30

atscott reviewed Apr 20, 2022

View reviewed changes

AndrewKushnir approved these changes Apr 20, 2022

View reviewed changes

pullapprove bot requested a review from atscott April 20, 2022 20:59

atscott approved these changes Apr 20, 2022

View reviewed changes

alxhub added action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note labels Apr 20, 2022

AndrewKushnir approved these changes Apr 20, 2022

View reviewed changes

dylhunn removed their request for review April 20, 2022 22:08

pullapprove bot requested a review from dylhunn April 20, 2022 22:08

alxhub and others added 6 commits April 20, 2022 15:21

test(core): add more tests for the standalone injector

089ee93

This commit reorganizes the tests around the EnvironmentInjector and its use for standalone injectors, and adds a number of new test cases.

docs(core): add API documentation to standalone-related classes

d007876

This commit adds some internal documentation.

alxhub force-pushed the standalone branch from 94d2c8c to d007876 Compare April 20, 2022 22:21

atscott closed this in bb6e11c Apr 20, 2022

atscott pushed a commit that referenced this pull request Apr 20, 2022

docs(core): add API documentation to standalone-related classes (#45687)

9e4c4bc

This commit adds some internal documentation. PR Close #45687

AndrewKushnir mentioned this pull request May 11, 2022

refactor(core): avoid code duplication in standalone-related logic #45949

Closed

3 tasks

alxhub deleted the standalone branch May 19, 2022 22:47

angular-automatic-lock-bot bot locked and limited conversation to collaborators Jun 19, 2022

	const ngModuleDef: NgModuleDef<any>\|null = getNgModuleDef(dep);
	const ngModuleDef = getNgModuleDef<Type<unknown>>(dep);

		// skip verifications of standalone components, direcrtives and pipes
		if (isStandalone(moduleType)) return;

Conversation

alxhub commented Apr 19, 2022

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AndrewKushnir left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

atscott left a comment

Choose a reason for hiding this comment

Uh oh!

alxhub commented Apr 20, 2022

Uh oh!

AndrewKushnir left a comment

Choose a reason for hiding this comment

Uh oh!

atscott commented Apr 20, 2022

Uh oh!

angular-automatic-lock-bot bot commented Jun 19, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants