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

EmberApp#styles should merge with overwrite #3930

Closed
lukemelia opened this issue Apr 20, 2015 · 3 comments
Closed

EmberApp#styles should merge with overwrite #3930

lukemelia opened this issue Apr 20, 2015 · 3 comments

Comments

@lukemelia
Copy link
Contributor

When javascript trees are merged (addons, external, app, etc), overwrite: true is passed (https://github.com/ember-cli/ember-cli/blob/master/lib/broccoli/ember-app.js#L531)

The same thing should be true for styles, but it's not currently. https://github.com/ember-cli/ember-cli/blob/master/lib/broccoli/ember-app.js#L955

I believe the semantics we want for addons in terms of contents of app/ is "last one wins", and app should win out over addons, so merging with overwrite: true is an appropriate solution here.

The practical application that led me to this is that I have an addon "A" with nested addon "B", and addon "A" will not build because a mergeTrees is choking on a conflict with a "B"'s app/styles/ dir. However there is not actually a conflict. It appears that "A"'s dummy app is including the tree from "B" twice, once as a dependency of "A" and once as a dependency of itself, and this leads to the conflict. My suggestion above does not address this oddity, but it does make it less painful.

@lukemelia
Copy link
Contributor Author

I'm happy to do the work on this if I get a greenlight from the maintainers.

@stefanpenner
Copy link
Contributor

Yes app/*/ Should merge, the only requirement is that stuff in the primary app takes presedence

rwjblue added a commit that referenced this issue Apr 21, 2015
[BUGFIX] Merge app/styles from addons with overwrite: true. Fixes #3930.
@rwjblue
Copy link
Member

rwjblue commented Apr 21, 2015

Closed by #3937.

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

No branches or pull requests

3 participants