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

vite: do not optimize dynamic addons #1850

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

patricklx
Copy link
Contributor

@patricklx patricklx commented Mar 21, 2024

with dynamic addons I mean addons which override the treeFor methods.
they can potentially depend on app tree and rebuild whenever an app file changes. e.g ember-svg-jar, ember-cli-addon-docs.

With dep optimization that will not be updated. they need to be excluded

@patricklx patricklx marked this pull request as ready for review March 22, 2024 10:23
@patricklx patricklx changed the title do not optimize dynamic addons vite: do not optimize dynamic addons Mar 22, 2024
Copy link
Member

@mansona mansona left a comment

Choose a reason for hiding this comment

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

So I am pretty sure that we don't want to go in this direction. A lot of thought has gone into the V2 addon spec to define what should or shouldn't be possible. This sort of addition to the spec feels like it should be a new RFC 🙈 but at the very least it should be discussed at a Tooling team meeting before we move forward.

I can see what you're trying to do here, you're trying to keep track of auto-upgraded addons that have customised their trees, but any feature that you add to the v2 addon spec could theoretically be consumed directly by a v2 addon and there is no way to prevent that and limit it to auto-upgraded v1 addons only.

If you want to discuss this idea further I recommend adding an Agenda item to the Tooling meeting or bringing it up at office-hours 👍

@patricklx
Copy link
Contributor Author

I actually have to re verify this with #1876 first if this is still an issue.
Maybe i only had bad dep optimizer issues.

Can v2 addons also override trees? Then they also would need this.
The issue was currently with v1 addons that merge into themselves the app tree and do some app files processing. They create new files or recreate files based on app filed during dev. And if they are optimized, vite wouldn't re-optimize them. But that's also an issue without dynamic addons. Vite wouldn't optimize when a previous unused dep was imported during development

@NullVoxPopuli
Copy link
Collaborator

Can v2 addons also override trees?

they cannot 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants