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(entities-*): externalize deps #1579

Merged
merged 17 commits into from
Sep 4, 2024
Merged

Conversation

adamdehaven
Copy link
Member

@adamdehaven adamdehaven commented Aug 23, 2024

Summary

Update the build configuration of entities-* packages to externalize common dependencies.

Externalizes:

  • @kong-ui-public/entities-shared
  • @kong/icons

Switches the Kongponents external to utilize a Regular Expression

The .npmrc changes need to be replicated in konnect-ui-apps

Before

dist/style.css                      11.06 kB │ gzip:   2.37 kB
dist/entities-certificates.es.js   504.87 kB │ gzip: 111.05 kB
dist/style.css                      11.06 kB │ gzip:  2.37 kB
dist/entities-certificates.umd.js  363.66 kB │ gzip: 94.92 kB

After

dist/style.css                      11.06 kB │ gzip:  2.37 kB
dist/entities-certificates.es.js   336.60 kB │ gzip: 69.73 kB
dist/style.css                      11.06 kB │ gzip:  2.37 kB
dist/entities-certificates.umd.js  246.57 kB │ gzip: 60.10 kB

Before

before

After

after

Consuming PR in konnect-ui-apps

@adamdehaven adamdehaven changed the title feat(entities-certificates): externalize deps feat(entities-*): externalize deps Aug 26, 2024
@ValeryG
Copy link
Collaborator

ValeryG commented Aug 29, 2024

what we probably should also do is to look at every package and change those full overrides of externals to "append to what is in shared" logic?

https://github.com/Kong/public-ui-components/blob/main/packages/entities/entities-routes/vite.config.ts#L24

@@ -19,11 +19,7 @@ const config = mergeConfig(sharedViteConfig, defineConfig({
},
rollupOptions: {
external: [
'@kong-ui-public/entities-shared',
'@kong-ui-public/entities-shared/dist/style.css',
Copy link
Member Author

Choose a reason for hiding this comment

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

@ValeryG @2eha0 we are not externalizing the styles anymore as a consuming application fails to resolve:

FAIL  src/stores/tests/sidebar.spec.ts [ src/stores/tests/sidebar.spec.ts ]
TypeError: Unknown file extension ".css" for /Users/val.gorodnichev@konghq.com/Code/Kong/ui/konnect-ui-apps/node_modules/.pnpm/@kong-ui-public+entities-shared@3.7.8-pr.1579.613e3228.0_@kong-ui-public+i18n@2.2.2_typescrip_p6jtyje3fjnz4ykopxijenw6pq/node_modules/@kong-ui-public/entities-shared/dist/style.css
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'ERR_UNKNOWN_FILE_EXTENSION' }
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯

 Test Files  1 failed (1)

* fix: more vite.config cleanups

* Update packages/entities/entities-routes/vite.config.ts

Co-authored-by: Adam DeHaven <2229946+adamdehaven@users.noreply.github.com>

* Update packages/analytics/analytics-geo-map/vite.config.ts

Co-authored-by: Adam DeHaven <2229946+adamdehaven@users.noreply.github.com>

* Update vite.config.ts

* Update vite.config.ts

---------

Co-authored-by: Adam DeHaven <2229946+adamdehaven@users.noreply.github.com>
@adamdehaven adamdehaven marked this pull request as ready for review September 3, 2024 15:23
@adamdehaven adamdehaven requested review from kongponents-bot and a team as code owners September 3, 2024 15:23
@adamdehaven adamdehaven requested review from a team and jillztom as code owners September 3, 2024 15:23
@adamdehaven adamdehaven merged commit 49a30f0 into main Sep 4, 2024
34 checks passed
@adamdehaven adamdehaven deleted the feat/entities-packaging branch September 4, 2024 17:21
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.

5 participants