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

Separate Angular production vs non-production entries #887

Merged
merged 1 commit into from
Jan 6, 2025

Conversation

davidlj95
Copy link
Contributor

@davidlj95 davidlj95 commented Dec 20, 2024

This is a work in progress. #886 should be merged first to take that into account too. #884 should be merged first too in order for production mode to run.

Some of the entry files added as part of Angular plugin are added as production entries. However they may not be production entries. Hence causing production mode to not work as expected.

Configuration for building an Angular project is usually specified under the build target inside the architect object of the Angular CLI workspace (aka angular.json). However, right now the implementation is using configuration options from all targets. Which is good to capture also build options like when building for testing within the test target. However, in that scenario, the entries added should be non-production ones.

Also, in any architect target options, the options defined in there are the default values for that target. But they may be overridden when specifying one or more alternative configurations. From those, the production configuration will contain production options and the rest are very probably non-production ones.

Here are the options from which we're adding entries and the Angular CLI's builders they can be used with

Option Webpack Karma (test) ESBuild browser ESBuild app (@angular-devkit) / ESBuild app (@angular/build)
main X X
scripts X X X X
fileReplacements X X X X
browser X
server X
ssr.entry X

This PR does two main things:

  1. Refactors to read entry files/patterns from both default options and also from each of the configurations defined. This is an additional feature, as previously entries defined in configurations weren't read (apart from fileReplacement ones)
  2. Add as production entries only the entries for the build target in production configuration. Takes into account that if an option isn't defined in the production configuration (if any), the default option will be used instead. Rest of entries will be development ones

Also the order of options has been changed so that they appear more or less chronologically ascendent. For instance, main option comes from the classic / old Webpack builder so appears first than newer browser option in application builder.

@davidlj95 davidlj95 changed the title Separate Angular production vs non-production entries Separate Angular production vs non-production entries [wip] Dec 20, 2024
Copy link

pkg-pr-new bot commented Dec 20, 2024

Open in Stackblitz

npm i https://pkg.pr.new/knip@887

commit: c9b3416

@davidlj95 davidlj95 force-pushed the angular-prod-vs-non-prod-entries branch from 105f725 to 979020d Compare December 20, 2024 17:16
@davidlj95 davidlj95 force-pushed the angular-prod-vs-non-prod-entries branch from 979020d to 1d92982 Compare December 23, 2024 17:13
@davidlj95 davidlj95 marked this pull request as ready for review December 23, 2024 17:36
@davidlj95 davidlj95 changed the title Separate Angular production vs non-production entries [wip] Separate Angular production vs non-production entries Dec 23, 2024
@davidlj95 davidlj95 force-pushed the angular-prod-vs-non-prod-entries branch from 1d92982 to c9b3416 Compare December 23, 2024 17:37
Copy link
Collaborator

@webpro webpro left a comment

Choose a reason for hiding this comment

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

Thanks a bunch, David. This is all great, just a nitpick left.

packages/knip/test/plugins/angular3.test.ts Show resolved Hide resolved
@webpro webpro merged commit 91130f8 into webpro-nl:main Jan 6, 2025
23 checks passed
@webpro
Copy link
Collaborator

webpro commented Jan 6, 2025

Just went ahead and merged to prepare for the next release. Thanks a bunch again, David!

@davidlj95 davidlj95 deleted the angular-prod-vs-non-prod-entries branch January 7, 2025 15:52
@webpro
Copy link
Collaborator

webpro commented Jan 9, 2025

🚀 This pull request is included in v5.42.0. See Release 5.42.0 for release notes.

Using Knip in a commercial project? Please consider becoming a sponsor.

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.

2 participants