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

refactor: vendor used typescript ast utils from @schematics/angular #18155

Conversation

@devversion
Copy link
Member

devversion commented Jan 11, 2020

Instead of directly accessing utility functions from @schematics/angular, we should
vendor the AST utils we need. This is necessary because @schematics/angular vendors
TypeScript, and this causes a type mismatch if we try to pass in ts.Node's which are
created from the TypeScript version available to @angular/cdk/schematics.

Using the vendored TS version from @schematics/angular is not an
option as it complicates our setup, gives us less control, and will
be hard to consistently enforce in the schematic code. We tried
to do this with the version agnostic typescript code, but it
had downsides in terms of type safety and convenience.

Note: Marked as P2 as it is needed to unblock angular/angular#33717

Instead of directly accessing utility functions from `@schematics/angular`, we should
vendor the AST utils we need. This is necessary because `@schematics/angular` vendors
TypeScript, and this causes a type mismatch if we try to pass in `ts.Node`'s which are
created from the TypeScript version available to `@angular/cdk/schematics`.

Using the vendored TS version from `@schematics/angular` is not an
option as it complicates our setup, gives us less control, and will
be hard to consistently enforce in the schematic code. We tried
to do this with the `version agnostic typescript` code, but it
had downsides in terms of type safety and convenience.
@devversion devversion requested review from jelbourn and angular/dev-infra-components as code owners Jan 11, 2020
@googlebot googlebot added the cla: yes label Jan 11, 2020
Copy link
Member

jelbourn left a comment

LGTM

@jelbourn jelbourn merged commit 8aa38ac into angular:master Jan 13, 2020
12 checks passed
12 checks passed
ci/angular: merge status All checks passed!
ci/circleci: api_golden_checks Your tests passed on CircleCI!
Details
ci/circleci: bazel_build Your tests passed on CircleCI!
Details
ci/circleci: build_release_packages Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: ngcc_compatibility Your tests passed on CircleCI!
Details
ci/circleci: tests_browserstack Your tests passed on CircleCI!
Details
ci/circleci: tests_local_browsers Your tests passed on CircleCI!
Details
ci/circleci: tests_saucelabs Your tests passed on CircleCI!
Details
ci/circleci: view_engine_test Your tests passed on CircleCI!
Details
cla/google All necessary CLAs are signed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.