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

enforce no app re-exports for engines #846

Conversation

void-mAlex
Copy link
Collaborator

setting an explicit treeForApp to an no-op function will ensure that no engine components leak into host app

some context for this PR:
the way to scaffold files in an engine is the same as an addon which is the conventional way to ember generate component|helper|util ...
this has the effect of adding files to the app re-export folder

because this doesn't cause an obvious failure in the app everything still works it's likely to be missed by users and committed to the repository
the immediate non-obvious effect of this is that engine components will be directly usable in a host app no matter the type of engine
this can lead to the re-exports being included twice in the build

This change makes it so there is no way for those files to make it into the build and brings the functionality in line with the documentation which states and so anything placed into the app directory of an Engine Addon will not be included in the build.

This change is important for the embroider initiative with its static analysis of the all components in an app/engines/addons as we try to split the build into the correct bundles as we follow what ember-engines does today and to keep correctness of what the build does under the classic pipeline we adhere to the app-re-exports if any are present

Copy link
Member

@villander villander left a comment

Choose a reason for hiding this comment

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

Great work! Thanks

@villander
Copy link
Member

@runspired @SergeAstapov could you please merge and release it?

@SergeAstapov SergeAstapov merged commit ef30f56 into ember-engines:master Aug 7, 2023
7 of 10 checks passed
@github-actions github-actions bot mentioned this pull request Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants