Skip to content
This repository has been archived by the owner on Dec 20, 2018. It is now read-only.

URL resolver shouldn't be managing its own prepending #58

Closed
bcardarella opened this issue Jul 6, 2017 · 6 comments
Closed

URL resolver shouldn't be managing its own prepending #58

bcardarella opened this issue Jul 6, 2017 · 6 comments
Labels

Comments

@bcardarella
Copy link

ember-cli will compose all the URLs you need, there isn't any need to manage this within ember-web-app: https://github.com/san650/ember-web-app/blob/master/lib/apple-link-tags.js#L34

When allowing ember-cli to handle the URL composition you end up with values like this:

<link rel="apple-touch-icon" href="https://assets.dockyard.com/https://assets.dockyard.com/assets/images/badge192-5bfe81121b638c30dc2fe97375428477.png" sizes="192x192">

@bcardarella
Copy link
Author

I suspect this is related: #22

@san650
Copy link
Owner

san650 commented Jul 8, 2017

@bcardarella I've made some changes to the addon and dropped the rename feature that was causing more problems than giving real benefits.

You can see these changes in PR #59. Can you check if these changes fixes the issues you're facing?

If this PR doesn't fix the problems, Can you add a bit more info what rootURL and broccoli-asset-rev configurations are you using?

@san650 san650 added the bug label Jul 8, 2017
@bcardarella
Copy link
Author

bcardarella commented Jul 8, 2017

I feel you're still working outside of ember-cli. Why not just add the necessary files to the /assets/ tree and allow ember-cli to handle all of the fingerprinting?

@san650
Copy link
Owner

san650 commented Jul 8, 2017

@bcardarella can you expand a bit the idea?

I'm trying to use ember-cli as much as possible and let ember-cli to handle the fingerprinting with broccoli-asset-rev.

There are some "things" I have to do because they are not done automatically by ember-cli, like using the rootURL config when rendering the meta tags. Note that rootURL is not part of broccoli-asset-rev.

Another feature that this addon brings is that it configures broccoli-asset-rev to process images inside manifest.webmanifest file without having the user to configure it... trying to do the "the right thing" out of the box.

Maybe there's something the addon is doing that can be avoided, I'm more than glad on removing/refactor that but It's not clear to me what would that be.

Really appreciate you looking into how the addon work and the feedback 😃

@bcardarella
Copy link
Author

@san650 no worries, we are making use of it so I'm happy to provide feedback :)

@san650
Copy link
Owner

san650 commented Nov 15, 2017

I'll close this issue for now, feel free to reopen it if you have more info about it.

@san650 san650 closed this as completed Nov 15, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants