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

feat: new builder impl, new config schematic #120

Merged
merged 2 commits into from
Aug 27, 2020
Merged

Conversation

JamesHenry
Copy link
Member

@JamesHenry JamesHenry commented Aug 27, 2020

This PR completely refactors the implementation of builder to address the performance and memory issues that have been raised in multiple issues.

I generated a number of additional projects for the integration-test workspace and was able to reproduce the issues pretty consistently so this was good confirmation that the initial "get it working" implementation of the builder needed improving.

In fact the "get it working" origins of this meant that initially the builder was just a port of the TSLint builder from angular-devkit. That builder (and the one this PR is replacing) creates one (or usually multiple) full TypeScript Program(s) each time it runs in order to figure out what files should be involved in the linting run. Not only is this expensive, it is also work that is doomed to be repeated, because ultimately ESLint is going to be invoking typescript-eslint which will need to build a TypeScript Program to represent the files of the run as well. There is no way to share program data between these runs.

Therefore the fundamental change of approach here is to use the builder as a very lightweight wrapper around ESLint, and then let typescript-eslint do the heavy lifting it needs to anyway.

To maximise performance and memory consumption, and ensure that all editor tooling etc, works as expected a new tsconfig.eslint.json file per project will be used to precisely control what goes into the linting-related compilation.

To mitigate any user pain around creating this extra config file, I have also included a new schematic in this PR which handles creating this automatically.

I have increased the detail and focus of the docs, and cover the updated process of migration from TSLint to ESLint.

After implementing these changes the multi-project integration-tests now run much faster and I have not seen any issues with OOM errors.


Because this is a major change it will be released as v0.2.0-beta.1

@codecov
Copy link

codecov bot commented Aug 27, 2020

Codecov Report

Merging #120 into master will decrease coverage by 0.24%.
The diff coverage is 89.28%.

@@            Coverage Diff             @@
##           master     #120      +/-   ##
==========================================
- Coverage   91.04%   90.80%   -0.25%     
==========================================
  Files          48       48              
  Lines        1128     1087      -41     
  Branches      216      206      -10     
==========================================
- Hits         1027      987      -40     
+ Misses         65       61       -4     
- Partials       36       39       +3     
Impacted Files Coverage Δ
packages/schematics/src/ng-add/index.ts 88.46% <ø> (ø)
packages/builder/src/utils/eslint-utils.ts 81.81% <50.00%> (-14.19%) ⬇️
...ages/schematics/src/add-config-to-project/utils.ts 82.14% <82.14%> (ø)
packages/builder/src/index.ts 92.30% <100.00%> (-0.55%) ⬇️
...ages/schematics/src/add-config-to-project/index.ts 100.00% <100.00%> (ø)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant