Skip to content

feat(builder): swap angular builder based on the angular version#22

Merged
NachoVazquez merged 19 commits into
mainfrom
feat/swap-angular-build
Apr 26, 2021
Merged

feat(builder): swap angular builder based on the angular version#22
NachoVazquez merged 19 commits into
mainfrom
feat/swap-angular-build

Conversation

@NachoVazquez

@NachoVazquez NachoVazquez commented Apr 6, 2021

Copy link
Copy Markdown
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our guidelines: CONTRIBUTING.md#commit
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • The latest artifacts are committed (run yarn build)

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

The action doesn't support changing the angular builder since only modifies the package.json

Fixes: #20

What is the new behavior?

The angular.json builder is modified depending on the angular version passed as input.

For version below 10.1.x it uses "@angular-devkit/build-ng-packagr:build"

For version greater or equal than 10.1.x it uses "@angular-devkit/build-angular:ng-packagr"

Does this PR introduce a breaking change?

[x] Yes
[ ] No

Other information

  • The input variable file-path is not root-path, it should be a folder instead of a path, it should be the folder where the package.json and angular.json exist and it defaults to "."
  • Unlike before the angular.json gets modify which can cause different outcomes on library compilations.

@NachoVazquez NachoVazquez requested a review from LayZeeDK April 6, 2021 01:47
@NachoVazquez NachoVazquez changed the title chore(deps): bump y18n from 4.0.0 to 4.0.1 feat(builder): swap angular builder based on the angular version Apr 6, 2021
Comment thread .github/ISSUE_TEMPLATE/bug_report.md Outdated
Comment thread __tests__/angular-json/mixed-angular-json.ts Outdated
Comment thread README.md Outdated
Comment thread __tests__/angular-json/old-builder-angular-json.ts Outdated
Comment thread __tests__/angular-json/newer-builder-angular-json.ts Outdated
Comment thread src/main.ts Outdated
Comment thread src/main.ts Outdated
Comment thread src/main.ts Outdated
Comment thread src/switch-angular-builder.ts Outdated
Comment thread src/switch-angular-builder.ts Outdated
@NachoVazquez NachoVazquez force-pushed the feat/swap-angular-build branch from 055ef79 to b507413 Compare April 10, 2021 00:46
@NachoVazquez NachoVazquez requested a review from LayZeeDK April 10, 2021 00:47
Comment thread README.md Outdated
Comment thread __tests__/angular-json/post10_1-library-builder-workspace.ts Outdated
Comment thread __tests__/angular-json/post10_1-library-builder-workspace.ts
Comment thread __tests__/angular-json/mixed-angular-json.ts Outdated
Comment thread src/types/angular-json.ts Outdated
Comment thread action.yml Outdated
Comment thread src/main.ts Outdated
Comment thread src/main.ts Outdated
Comment thread src/main.ts Outdated
Comment thread __tests__/switch-angular-builder.test.ts Outdated
@NachoVazquez NachoVazquez requested a review from LayZeeDK April 11, 2021 03:21
Comment thread __tests__/angular-json/regular-angular-json.ts Outdated
Comment thread src/replace-libraries-ngpackagr-builder.ts Outdated
Comment thread src/replace-libraries-ngpackagr-builder.ts Outdated
Comment thread src/replace-libraries-ngpackagr-builder.ts
test(`when using the version ${version} it should replace all libraries build builder with ${pre10_1LibraryBuilder}`, () => {
const actual = replaceLibrariesBuildBuilder(version, mixedAngularJson);

expect(actual).toEqual(pre10_1LibraryBuilderWorkspace);

@LayZeeDK LayZeeDK Apr 18, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What I mean is let the test show that the builder name is what's changed. This is not currently clear in the test body, only in the test description. Assert on the builder name of a library build architect target, not the entire workspace configuration.

@NachoVazquez

Copy link
Copy Markdown
Contributor Author

What I mean is let the test show that the builder name is what's changed. This is not currently clear in the test body, only in the test description. Assert on the builder name of a library build architect target, not the entire workspace configuration.

I know what you are saying but the reason I like to do it this way is that it gives me confidence that I'm not changing other target builders that I should not be changing.
Just asserting against the builder that should change is not covering that kind of errors. The solution could be checking builder by builder, I don't like that, but I would wait for your thoughts on this one.

@NachoVazquez NachoVazquez requested a review from LayZeeDK April 19, 2021 02:48
Comment thread src/replace-libraries-ngpackagr-builder.ts Outdated
Comment thread __tests__/angular-json/lumberjack-mixed-angular-json.ts Outdated
Comment thread src/angular-version-comparer.ts Outdated
Comment thread __tests__/replace-libraries-ngpackagr-builder.test.ts
@NachoVazquez NachoVazquez requested a review from LayZeeDK April 25, 2021 14:12
Comment thread src/replace-libraries-ngpackagr-builder.ts Outdated
Comment thread src/replace-libraries-ngpackagr-builder.ts
Comment thread src/replace-libraries-ngpackagr-builder.ts Outdated
Comment thread src/replace-libraries-ngpackagr-builder.ts Outdated
Comment thread src/angular-version-comparer.ts Outdated
Comment thread __tests__/replace-libraries-ngpackagr-builder.test.ts Outdated
Comment thread __tests__/replace-libraries-ngpackagr-builder.test.ts Outdated
@NachoVazquez NachoVazquez requested a review from LayZeeDK April 25, 2021 22:07
Comment thread src/angular-version-comparer.ts Outdated
Comment thread src/replace-libraries-ngpackagr-builder.ts Outdated
Comment thread src/angular-version-comparer.ts Outdated
@NachoVazquez NachoVazquez force-pushed the feat/swap-angular-build branch from cbfdf5b to 8e1669f Compare April 26, 2021 21:03
@NachoVazquez NachoVazquez requested a review from LayZeeDK April 26, 2021 21:03
Co-authored-by: Lars Gyrup Brink Nielsen <larsbrinknielsen@gmail.com>
@NachoVazquez NachoVazquez merged commit 51246c5 into main Apr 26, 2021
@NachoVazquez NachoVazquez deleted the feat/swap-angular-build branch April 26, 2021 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

fix: support < v10.1.0-next.6 library projects

2 participants