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.

Sign up for GitHub

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

Jump to bottom

strapi-plugin-navigation/dist/admin/index.js/mjs contains entire lodash source #509

Closed
jorrit opened this issue Jan 24, 2025 · 7 comments
Closed

strapi-plugin-navigation/dist/admin/index.js/mjs contains entire lodash source #509

jorrit opened this issue Jan 24, 2025 · 7 comments
Labels
dependencies Pull requests that update a dependency file next release Planned for upcoming release solved Issue has been solved v5 Strapi v5

Comments

@jorrit
Copy link
Contributor

jorrit commented Jan 24, 2025

I noticed that the files in the dist/admin folder are pretty big: 2MB for js and mjs. It seems that for some reason lodash is bundled in these files:

Image

I tried to find the cause in @strapi/sdk-plugin and @strapi/pack-up but I could not find it quickly. Perhaps you have an idea.
@cyp3rius
Copy link
Contributor

We will investigate it, maybe the case is with bundling as a devDependency.

cyp3rius added a commit that referenced this issue Jan 24, 2025
@cyp3rius
fix: #509 pkg size optimizations
920b1bd
@cyp3rius cyp3rius mentioned this issue Jan 24, 2025
Merged
@cyp3rius cyp3rius added solved Issue has been solved dependencies Pull requests that update a dependency file v5 Strapi v5 next release Planned for upcoming release labels Jan 24, 2025
@cyp3rius
Copy link
Contributor

Optimized package released as v3.0.7-beta.1. Validate please and let us know if all works as expected.

@jorrit
Copy link
Contributor Author

jorrit commented Jan 24, 2025

I still find lodash in the code, albeit minified:

Image

I think the source is somewhere deep in @strapi/pack-up and / or @strapi/sdk-plugin.

@cyp3rius
Copy link
Contributor

Most probably so that's an issue to address for Strapi Core team :) Last changes made package twice lighter so should be more manageable.

@jorrit
Copy link
Contributor Author

jorrit commented Jan 24, 2025

I see you minified the source. I don't think that that is useful, because the plugin will be bundled and minified again when it is integrated in a Strapi application.

@jorrit jorrit mentioned this issue Jan 24, 2025
Open
@cyp3rius
Copy link
Contributor

I see you minified the source. I don't think that that is useful, because the plugin will be bundled and minified again when it is integrated in a Strapi application.

As the NPM package is not carry the source files (TS) the minified source is going to be injected into the Strapi build, the size shouldn't be impacted.

cyp3rius added a commit that referenced this issue Jan 27, 2025
@cyp3rius
fix: #509 pkg size optimizations (#510)
5eeec55 
* fix: #509 pkg size optimizations

* chore: beta version bump
@jorrit
Copy link
Contributor Author

jorrit commented Jan 29, 2025

I have looked into this a bit and it seems it is not lodash that is included, but the entire strapi design system code. The index.js / index.mjs files of strapi design system contain lodash and for some reason these files are embedded in your plugin instead of imported.

This was referenced Jan 29, 2025
Merged
Merged
cyp3rius added a commit that referenced this issue Feb 1, 2025
@cyp3rius
fix: #509 pkg size optimizations (#510)
7f847a8 
* fix: #509 pkg size optimizations

* chore: beta version bump
HichamELBSI added a commit to strapi/design-system that referenced this issue Feb 3, 2025
@HichamELBSI
Register lodash as dependency to externalize it (#1844)
09dc2ec 
### What does it do?

I noticed that the dist files of the design system package contain the
entire lodash source.

See:
https://www.npmjs.com/package/@strapi/design-system/v/2.0.0-rc.14?activeTab=code

![image](https://github.com/user-attachments/assets/26e8220f-bdeb-4188-935c-483037715b11)

The design-system package uses lodash directly in extendTheme.ts but
doesn't list this as a dependency in package.json. lodash happens to be
present because it is a sub-sub-subdependency:

```
> npm ls lodash
[email protected] .\strapi-design-system
`-- @strapi/[email protected] -> .\packages\design-system
  `-- @strapi/[email protected]
    `-- [email protected]
      `-- [email protected]
```

By making it a direct dependency, vite will not bundle it in the dist
files. Besides that, it is more correct to not rely on sub-dependencies.

### Why is it needed?

When investigating the dist code of the strapi-plugin-navigation module
I noticed that it contains the entire lodash code. I investigated this
and it lead to the conclusion that it is actually the design-system
package that provides this.
See
VirtusLab-Open-Source/strapi-plugin-navigation#509

### How to test it?

Build the design-system package before and after this change and observe
the file size decrease.

_It may be the case that there are more dependencies of the code that
could be externalized_
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
dependencies Pull requests that update a dependency file next release Planned for upcoming release solved Issue has been solved v5 Strapi v5
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jorrit @cyp3rius