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

Remove "kadira:dochead" Meteor package #3548

Closed
aldeed opened this issue Jan 23, 2018 · 2 comments

Comments

Projects
None yet
4 participants
@aldeed
Copy link
Member

commented Jan 23, 2018

Work:

  • "kadira:dochead" Meteor package is used only in /lib/api/router/metadata.js, which runs only on the client, so we do not need isomorphic. Copy just the client parts of the DocHead object (and only the functions we use) into Reaction. Code is here: https://github.com/kadirahq/meteor-dochead/blob/master/lib/both.js
  • Remove the kadira:dochead Meteor package
  • Move all the metadata code into the core plugin within /imports

@aldeed aldeed added the performance label Jan 23, 2018

@jshimko

This comment has been minimized.

Copy link
Contributor

commented Jan 23, 2018

FYI, I created a little addTag helper to add any kind of tags to the head when I created the custom favicons stuff.

https://github.com/reactioncommerce/reaction/blob/master/imports/plugins/included/default-theme/client/favicons.js

Perhaps that can somehow be used or consolidated in this process. I only mention it because I happen to prefer the more flexible addTag function instead of making a method for each tag type like kadira:dochead did with... addLink(), addMeta(), addLdJsonScript()

Maybe we create a new Reaction.DOM namespace for this stuff?

Reaction.DOM.getTitle();
Reaction.DOM.setTitle(title);
Reaction.DOM.addHeadTag(type, attributes);
Reaction.DOM.loadScript(src, attributes, callback);

Or something. :)

@willopez

This comment has been minimized.

Copy link
Member

commented Jan 30, 2018

@aldeed @jshimko I agree with Jeremy, this is a good opportunity to consolidate document head utilities. If there are no objections, I'll move forward and create a new namespace, Reaction.DOM and move the relevant functions from the meteor package to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.