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

docs: clarify that barrel file issue is for apps rather libs #17651

Closed
wants to merge 3 commits into from

Conversation

benmccann
Copy link
Collaborator

@benmccann benmccann commented Jul 10, 2024

Description

Blu states in the issue that barrel file performance issue is for apps rather libs. Make that clearer in the docs.

#8237 (comment)

Copy link

stackblitz bot commented Jul 10, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

docs/guide/performance.md Outdated Show resolved Hide resolved
@patak-dev
Copy link
Member

Could you link to the issue you're referring?

@benmccann
Copy link
Collaborator Author

Oops. Sorry about that. I've updated the description with a link to the issue comment

@patak-dev
Copy link
Member

Thanks! I'm good with merging this. I'll let @bluwy do it to review the wording.

As a note, the only thing that may be interesting is how this affects dependencies that aren't pre-bundled. We talked with Anthony in the past that some deps would be better of avoiding prebundling (if they are esm and doesn't have many deps). But I imagine these would mainly be prebundled already like Vite.

@bluwy
Copy link
Member

bluwy commented Jul 15, 2024

I'm not really sure about this wording change. The docs page already implies that the suggestions only apply to a Vite app. The other sections don't quite make sense for library authors.

Even with that aside, avoiding barrel files in libraries is somewhat beneficial too. Not all environments can bundle them, e.g. you're using a very big icon library that exports all SVG metadata objects in SSR.

docs/guide/performance.md Outdated Show resolved Hide resolved
@benmccann
Copy link
Collaborator Author

The reason I was sending a PR to this page is because I saw people talking about whether or not barrel files should be avoided in libraries. I think it's better to be explicit about apps vs libraries here since this could apply to both. It's a pretty complicated question because there's a tradeoff between developer experience and performance. I know in the vite-plugin-svelte docs we have much more info on this topic, so I just updated it to add a link there

I've never seen it be a problem in SSR before. The dependency would probably be bundled and tree shaken in production where you might be most performance sensitive.

@bluwy
Copy link
Member

bluwy commented Jul 23, 2024

In SSR, dependencies are usually externalized so the barrel files affect the runtime. So indeed the advice applies to both libraries and apps, but in the context of this page, it still focuses on the app side. I don't think we need to change this section.

@benmccann
Copy link
Collaborator Author

In SSR, dependencies are usually externalized so the barrel files affect the runtime. So indeed the advice applies to both libraries and apps

You will pay a small hit parsing more code, but I'm not sure it's too meaningful on the server. Also, during build dependencies are usually bundled, so it's largely restricted to dev where that level of performance difference matters even less.

@benmccann benmccann closed this Jul 26, 2024
@benmccann benmccann deleted the benmccann-patch-2 branch July 26, 2024 21:44
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.

3 participants