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

Remove hbs dependency #1855

Merged
merged 4 commits into from
Jan 27, 2020
Merged

Conversation

Naktibalda
Copy link
Contributor

Reasons:

  • hbs view engine is registered, but not used anywhere.
  • handlebars library had 2 security vulnerabilies recently, but hbs is slow to update,

We discussed it in Slack last week:

benbrown 8:49 PM
It’s in the conversation system

naktibalda 10:03 AM
https://github.com/howdyai/botkit/blob/v4.6/packages/botkit/src/conversation.ts#L858 uses mustache, not handlebars
packages/botkit/src/conversation.ts:858

        outgoing.text = mustache.render(outgoing.text, { vars: vars });

naktibalda 10:12 AM
The only package using hbs templates is docs, https://github.com/howdyai/botkit/tree/master/packages/docs
naktibalda 10:19 AM
hbs isn't used by botkit, but it could be used by third-party plugins so removing it is a BC-break and requires major version bump. Not worth it. (edited)
benbrown 2:39 PM
Ah I think it would be safe
Plugins need to declare their own deps (edited)

To make the next change clearer
because it isn't used by botkit
@benbrown benbrown self-assigned this Nov 12, 2019
@benbrown benbrown added this to the 4.6.+ milestone Nov 12, 2019
@Naktibalda
Copy link
Contributor Author

hbs: 4.1.0 was released today,
it is necessary to either remove it from dependencies or update version constraint to allow upgrade to 4.1.0.

@Naktibalda
Copy link
Contributor Author

My previous comment was incorrect, there is no need to update version constraint to get a new version of hbs.
It would still be useful to remove this dependency.

@benbrown
Copy link
Contributor

I'm planning on building a new release as soon as #1894 is fixed.

@benbrown benbrown merged commit a108c6c into howdyai:master Jan 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants