Removing uses of "computeNode()" from the compiler.

[matanlurey]

In a future period of time, running <Element>.computeNode() will be prohibited.

Already it is a O(n^2) operation, and has many edge cases (not provided for summaries).

We've cut down our use significantly already:
https://github.com/dart-lang/angular/search?utf8=%E2%9C%93&q=computeNode&type=

Here are cases active today:

• Getting a list of all directives in a LibraryElement
  NOTE: We can't use the .directives list directly, because it does not include imports/exports
  that were not resolved, sometimes because they just aren't generated yet. We need to validate if
  this is still necessary. If Compiler URL resolution of types is sometimes invalid #740 is fixed (and we see all imports, even invalid ones), we could easily
  remove this use of .computeNode().

• Support arbitrary tokens in @Inject(...).
  I don't know how much this is used today, but we plan on only support Type and tokens in a
  future point anyway, so maybe this is a forcing function to remove other arbitrary constants used.

• Supporting the legacy router.
  This will take several months/quarters to remove entirely, but maybe we can change this part now
  to use the element model. I'll experiment with this and see if we can just scratch this one off.

/cc @alorenzen
/cc @devoncarew @MichaelRFairhurst @scheglov for FYI.

[scheglov]

You get all directives in a LibraryElement.
I wrote a small test into the AnalysisDriver test suite:

  test_getResult_unresolvedUri() async {
    addTestFile(r'''
import 'package:foo/bar.dart';
export 'package:foo/baz.dart';
''');

    AnalysisResult result = await driver.getResult(testFile);
    var libraryElement = result.unit.element.library;
    expect(libraryElement.imports[0].uri, 'package:foo/bar.dart');
    expect(libraryElement.imports[0].importedLibrary, isNull);
    expect(libraryElement.exports[0].uri, 'package:foo/baz.dart');
    expect(libraryElement.exports[0].exportedLibrary, isNull);
  }

I don't understand the second item, but it might be something internal to Angular. Let me know if this is something with what you need assistance from the Analyzer team.

[matanlurey]

@scheglov:

You get all directives in a LibraryElement ... AnalysisDriver

We still use AnalysisContext, and we don't (#740). If we change build to use driver, sure.

@natebosch @jakemac53

I don't understand the second item, but it might be something internal to Angular.

It is!

https://github.com/dart-lang/angular/blob/c1331cd80334ced1b260c3746a7b2b40c1d824db/angular_compiler/lib/src/analyzer/di/tokens.dart#L58-L67

Similar here:

https://github.com/dart-lang/angular/blob/c1331cd80334ced1b260c3746a7b2b40c1d824db/angular_compiler/lib/src/emitter/reflector.dart#L353-L361

[scheglov]

@matanlurey See https://dart-review.googlesource.com/c/sdk/+/40600 for keeping directives with unresolved URIs.

[matanlurey]

@scheglov Wow thank you so much!

[matanlurey]

@scheglov @MichaelRFairhurst

Can I get some confirmation we don't need this for Dart 2, or is this blocking Dart 2?

[MichaelRFairhurst]

My understanding is that computeNode() may lag behind dart 2/behave inconsistently with dart 2/become less stable as dart 2 progresses. So I don't think it blocks dart 2, but I think it may become a point of stability for you guys at that point. But I will have to defer to @scheglov on this

[scheglov]

Yes, AFAIK we don't need all users to switch to AnalysisDriver, but we might get results that are less close to Dart 2 than if you were using AnalysisDriver.

[matanlurey]

Thanks! I'm going to assume this is important, but not Dart2 beta-blocking important.

[matanlurey]

/cc @natebosch, because if you end up wanting to move build to analysis driver, this issue will become blocking. So delegating to you to determine if we need more movement here or not.

[natebosch]

We'll be taking a look across build package users for this at some point. We'll definitely make a lot of noise about it if it's going to become blocking.

[matanlurey]

Given @scheglov's changes, I'll try out removing the remaining .computeNode() calls!