Shields: Move badge templates from doT → Template literals

Created on 29 Nov 2018  Â·  9Comments  Â·  Source: badges/shields

:clipboard: Description
https://github.com/badges/shields/pull/2394#issuecomment-442651526
Rewrite the badge templates to use template literals.

:dart: Objective

  • Improve readability
  • Improve performance
  • Merge some parts of the templates to reduce duplication

:ballot_box_with_check: TODO

  • [ ] Write basic template
  • [ ] Test performance vs doT
  • [ ] Optimize
blocker npm-package

Most helpful comment

I'm making good progress on this. I should have a PR up sometime in the next couple of days.

It doesn't have exact parity right now, or run through the optimizer, so there is a bit of testing that needs to be done to make sure nothing is broken.

All 9 comments

Thanks for opening this.

I'm marking this "good first issue." This is not the most straightforward task, though for someone who is comfortable with JavaScript and unit testing, they probably would be able to accomplish this without a lot of knowledge of Shields. It'd be a good introduction to the project, and also very high value. And, of course it's a great thing for an existing contributor or maintainer to take on, too. 😄

I've made an example here using the converted templates:
Shields.io Badge Generator

Convert the doT template files to template strings:

template.replace('<svg', 'return `<svg')
  .replace(/\{\{=(.+?)\}\}/g, "${$1 ? `$2` : ''}")
  .replace(/\{\{\?(.+?)\}\}(.+?)\{\{\?\}\}/g, "${$1}")
  .replace(/\{\{\?(.+?)\}\}\r?\n(\s+)/g, "${$1 ?\n$2`")
  .replace(/\r?\n(\s+)\{\{\?\}\}/g, "`\n$1: ''}")
  .replace('</svg>', '</svg>`;');

Not sure how to go about it in shields though, will look into it tomorrow _(hopefully)_

What do you think about this approach? Placeholder logic, though it conveys the idea…

const hasLeftText = text[0].length > 0
const [leftWidth, rightWidth] = ...
const leftPadding = ...
const totalWidth = leftWidth + leftPadding + leftWidth

return `
<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="${totalWidth}" height="20">
...
`

I've updated the CodePen using a similar approach, Not sure how to tidy it up more though:
https://codepen.io/redsparr0w/full/PxgPdG/

Looks like progress!

Do you feel like playing with it further? I suppose the next things I'd suggest are destructuring it, and hoisting out more of the compound expressions like 10+it.logoWidth+it.logoPadding and leftWidth+rightWidth/2-(it.text[0].length ? 1 : 0 ))*10.

I've done some initial investigation into the possible performance improvements. From what I can tell there is not much to be gained here.

https://codepen.io/mbarkhau/pen/xMvvRd?editors=0010
https://jsperf.com/dot-vs-template-literals/1

Of course I may be doing an unfair comparison, but even if that is the case, I doubt that performance would be the main justification for this change vs the other objectives.

Thanks for looking into that! I think improving readability, DRY, and eliminating the mental overhead of another library are good reasons to do it.

Hey @RedSparr0w, mind if I pick this up?

I'm making good progress on this. I should have a PR up sometime in the next couple of days.

It doesn't have exact parity right now, or run through the optimizer, so there is a bit of testing that needs to be done to make sure nothing is broken.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

kerolloz picture kerolloz  Â·  3Comments

lukeeey picture lukeeey  Â·  3Comments

AlexWayfer picture AlexWayfer  Â·  3Comments

rominf picture rominf  Â·  3Comments

paulmelnikow picture paulmelnikow  Â·  3Comments