ci: add ts api guardian #12010

alan-agius4 · 2018-08-27T14:01:11Z

This adds the API guardian in this repo.

Not this doesn't cover schematics/* packages. As I'd prefer to tackle that separately as it will require a some more changes and the PR will be way bigger.

At the moment, the packages covered are;

@angular-devkit/architect
@angular-devkit/benchmark
@angular-devkit/build-optimizer
@angular-devkit/core
@angular-devkit/schematics

package.json

etc/api/index.bzl

alan-agius4 · 2018-08-28T06:39:18Z

DId some more fixes for bazel in angular/angular

Blocked by:
angular/angular#25694
#11889, This requires TS 3 due that the guardian is using TS3 as a dependency and when tsc_wrapped rules tries to resolve the TS libs it fails since libs references have changed in TS3 and will get errors such as

error TS2318: Cannot find global type 'Array'.
error TS2318: Cannot find global type 'Boolean'.
error TS2318: Cannot find global type 'Function'.
error TS2318: Cannot find global type 'IArguments'.
error TS2318: Cannot find global type 'Number'.
error TS2318: Cannot find global type 'Object'.
error TS2318: Cannot find global type 'RegExp'.

alexeagle · 2018-09-10T21:37:02Z

Is this ready to merge? Could you fix conflict?

filipesilva

The benchmark PR was also merged yesterday, and it includes a new package. It probably needs the same kind of codegen that was done in this PR.

packages/angular_devkit/architect/BUILD

packages/angular_devkit/core/src/experimental.ts

.circleci/config.yml

etc/api/angular_devkit/architect/src/index.d.ts

packages/angular_devkit/benchmark/src/interfaces.d.ts

etc/api/angular_devkit/benchmark/src/index.d.ts

alan-agius4 · 2018-09-14T07:57:38Z

Blocked by bazel-contrib/rules_nodejs#316 and bazel-contrib/rules_nodejs#317

alan-agius4 · 2018-11-02T15:13:41Z

Requires: angular/angular#26888

Been updated and made it work on Windows

alexeagle · 2018-11-13T00:25:47Z

angular/angular#26888 is green and marked for merge now

alan-agius4 · 2018-11-20T07:56:53Z

Marking as need further discussion to decide if we should go for api extractor.

…tion `ts-api-guardian` only support classic module resolution which means that we need to specify `index` so that the resolution works.

At the moment ts api guardian doesn't support aliased symbols as namespaces, this is a workaround to still have namespaced symbols in the final golden file.

filipesilva

The bits that I'm familiar with LGTM.

I still think we should add information about how the API guardian affects the developer story. From the CI config it looks like to test is just bazel test ....

Is there anything else that needs to be done as a developer, like updating some of the golden files or something?

alan-agius4 · 2018-11-27T12:39:07Z

@filipesilva you need to update the golden files you if modify the public API.

If you touched parts of the API that were aliased, you might need to add these exports in the _golden_api files, since these won't be automatically discovered by the guardian.

I can create a readme. Maybe in the contributing section?

hansl · 2018-11-27T23:38:56Z

packages/angular_devkit/core/node/fs.ts

-  } catch (e) {
-    if (e && (e.code === 'ENOENT' || e.code === 'ENOTDIR')) {
-      return false;
+export namespace fs {


We need a bigger discussion as export namespace ... is not considered best practices. See https://www.typescriptlang.org/docs/handbook/namespaces-and-modules.html#needless-namespacing

hansl · 2018-11-27T23:39:32Z

Need discussion as export namespace ... is not considered best practice and should be avoided.

hansl

Approving this to get moving.

alan-agius4 · 2018-11-28T05:30:11Z

One of the problems was that in some cases we have the same naming in 2 different "namespaces", and thus this would be that we cannot add it to the _golden_file unless they are properly namespace as they will collide.

If I recall correctly, it was terminal and colors.

angular-automatic-lock-bot · 2019-09-12T19:41:12Z

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.}

googlebot added the cla: yes label Aug 27, 2018

alan-agius4 changed the title ~~Build ts api guardian~~ ci: add ts api guardian Aug 27, 2018

alexeagle reviewed Aug 27, 2018

View reviewed changes

package.json Outdated Show resolved Hide resolved

alexeagle reviewed Aug 27, 2018

View reviewed changes

etc/api/index.bzl Outdated Show resolved Hide resolved

alan-agius4 added state: WIP state: blocked labels Aug 28, 2018

alan-agius4 added target: major This PR is targeted for the next major release and removed state: WIP state: blocked labels Sep 6, 2018

filipesilva previously requested changes Sep 11, 2018

View reviewed changes

packages/angular_devkit/architect/BUILD Show resolved Hide resolved

packages/angular_devkit/core/src/experimental.ts Show resolved Hide resolved

.circleci/config.yml Outdated Show resolved Hide resolved

etc/api/angular_devkit/architect/src/index.d.ts Show resolved Hide resolved

alan-agius4 commented Sep 11, 2018

View reviewed changes

packages/angular_devkit/benchmark/src/interfaces.d.ts Outdated Show resolved Hide resolved

filipesilva reviewed Sep 11, 2018

View reviewed changes

etc/api/angular_devkit/benchmark/src/index.d.ts Outdated Show resolved Hide resolved

alan-agius4 added the state: blocked label Sep 14, 2018

alan-agius4 added the state: WIP label Sep 26, 2018

alan-agius4 removed state: blocked state: WIP labels Nov 1, 2018

angular deleted a comment from ngbot bot Nov 3, 2018

alan-agius4 added needs: discussion On the agenda for team meeting to determine next steps PR state: blocked labels Nov 20, 2018

alan-agius4 removed PR state: blocked needs: discussion On the agenda for team meeting to determine next steps labels Nov 21, 2018

alan-agius4 requested a review from hansl November 21, 2018 13:32

refactor: fix import and export paths to work with classic resolu…

d13ace8

…tion `ts-api-guardian` only support classic module resolution which means that we need to specify `index` so that the resolution works.

alan-agius4 added 10 commits November 21, 2018 14:38

build: add npm_package to packages

6cca037

build: add ts-api-guardian to repo

72c770a

test: add api golden files

1dc4bb9

refactor: use proper namespace instead of alias export

1f2357c

refactor: use proper namspace einstead of alias export

1c336e6

build: add _golden_api files

d34c58e

At the moment ts api guardian doesn't support aliased symbols as namespaces, this is a workaround to still have namespaced symbols in the final golden file.

build: update angular archive for workspace

c1b47bd

test: fix reference to TestHost to use namespace

a1d9ba2

refactor: create fs namespace instead of aliased export

ee6a8b2

test: update api golden file for @angular-devkit/core/node

ea43163

filipesilva approved these changes Nov 27, 2018

View reviewed changes

alan-agius4 mentioned this pull request Nov 27, 2018

docs: add documentation for API guardian #13065

Merged

hansl reviewed Nov 27, 2018

View reviewed changes

hansl added the needs: discussion On the agenda for team meeting to determine next steps label Nov 27, 2018

hansl approved these changes Nov 28, 2018

View reviewed changes

hansl removed the needs: discussion On the agenda for team meeting to determine next steps label Nov 29, 2018

vikerman merged commit 717b02f into angular:master Nov 30, 2018

alan-agius4 deleted the build_ts_api_guardian branch November 30, 2018 19:58

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 12, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci: add ts api guardian #12010

ci: add ts api guardian #12010

alan-agius4 commented Aug 27, 2018 •

edited

Loading

alan-agius4 commented Aug 28, 2018 •

edited

Loading

alexeagle commented Sep 10, 2018

filipesilva left a comment

alan-agius4 commented Sep 14, 2018

alan-agius4 commented Nov 2, 2018

alexeagle commented Nov 13, 2018

alan-agius4 commented Nov 20, 2018

filipesilva left a comment

alan-agius4 commented Nov 27, 2018 •

edited

Loading

hansl Nov 27, 2018

hansl commented Nov 27, 2018

hansl left a comment

alan-agius4 commented Nov 28, 2018 •

edited

Loading

angular-automatic-lock-bot bot commented Sep 12, 2019

ci: add ts api guardian #12010

ci: add ts api guardian #12010

Conversation

alan-agius4 commented Aug 27, 2018 • edited Loading

alan-agius4 commented Aug 28, 2018 • edited Loading

alexeagle commented Sep 10, 2018

filipesilva left a comment

Choose a reason for hiding this comment

alan-agius4 commented Sep 14, 2018

alan-agius4 commented Nov 2, 2018

alexeagle commented Nov 13, 2018

alan-agius4 commented Nov 20, 2018

filipesilva left a comment

Choose a reason for hiding this comment

alan-agius4 commented Nov 27, 2018 • edited Loading

hansl Nov 27, 2018

Choose a reason for hiding this comment

hansl commented Nov 27, 2018

hansl left a comment

Choose a reason for hiding this comment

alan-agius4 commented Nov 28, 2018 • edited Loading

angular-automatic-lock-bot bot commented Sep 12, 2019

alan-agius4 commented Aug 27, 2018 •

edited

Loading

alan-agius4 commented Aug 28, 2018 •

edited

Loading

alan-agius4 commented Nov 27, 2018 •

edited

Loading

alan-agius4 commented Nov 28, 2018 •

edited

Loading