Skip to content
This repository has been archived by the owner on Feb 11, 2022. It is now read-only.

Figure out how to deal with favicon/apple icons etc. #78

Closed
bebraw opened this issue Jun 5, 2015 · 8 comments
Closed

Figure out how to deal with favicon/apple icons etc. #78

bebraw opened this issue Jun 5, 2015 · 8 comments

Comments

@bebraw
Copy link
Member

bebraw commented Jun 5, 2015

I suppose this should be a plugin. See https://github.com/haydenbleasel/favicons . Ideally we would just wrap that.

@eldh
Copy link
Member

eldh commented Jun 7, 2015

yeah. Do we need a nicer hook into the asset system?

@bebraw
Copy link
Member Author

bebraw commented Jun 7, 2015

Possibly.

Btw, check out jantimon/html-webpack-plugin#41. It feels like we could get a lot of features pretty much for free if we integrated html-webpack-plugin after that gets merged. Then we could piggyback on Webpack loaders etc. That favicons thing I linked to could be pushed into one for instance.

This would mean you would define a base template as a user for more advanced usage. We probably want to give it some thought before jumping into a solution but it's good have these kind of options.

@eldh
Copy link
Member

eldh commented Jun 7, 2015

Very interesting!

@jantimon
Copy link

jantimon commented Jun 9, 2015

Unfortunately https://github.com/haydenbleasel/favicons does not work offline

@bebraw
Copy link
Member Author

bebraw commented Jun 30, 2015

Yeah, that online requirement sucks. Using imagemagick bindings (unmaintained) a basic ico can be generated like this:

function toIco(input, output, cb) {
    im.convert([input,
        '-resize', 'x16',
        '-gravity', 'center',
        '-background', 'transparent',
        '-crop', '16x16+0+0',
        '-colors', '256',
        '-flatten',
        output
    ], cb);
}

It's probably easy to port to gm, a GraphicsMagick wrapper that seems to be maintained.

@eldh Given these operations are async maybe it would make sense to change extra hook to be async as well?

@eldh
Copy link
Member

eldh commented Jul 1, 2015

Will we gain anything in the end by making it async? In that case maybe all hooks should be async? If we use babel it should be fairly easy to do it with async functions?

@bebraw
Copy link
Member Author

bebraw commented Jul 1, 2015

That extra hook is used for generating files currently and it's run in Node context. Here are the relevant lines from build/build.js:

var pluginExtras = _.pluck(config.plugins, 'extra').filter(_.identity);
var extraFiles = _.map(pluginExtras, function(plugin) {
  return plugin(params.allPaths, config);
});

Technically speaking it would just become async.map instead and each hook would return its result through a callback whether it's sync or not. That would allow us to implement plugins that generate easily as often these operations are async.

I don't see any need to make all of the hooks async yet. Maybe someday but I can't think of a use case for that yet. This particular hook is an exception because the dependencies we might want to use here can be async by nature.

I wouldn't worry about babel yet. It's likely best to leave it out of Node code for now to keep things simple.

@bebraw
Copy link
Member Author

bebraw commented Nov 18, 2015

I pushed the problem to user level. Now you can override entire Body although I provide a starting point. This allows you to deal with favicons any way you want. For example, react-helmet could work well here.

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

No branches or pull requests

3 participants