Fix and refactor front-end URL generation

frontend/components/badge-examples.js

@@ -1,7 +1,7 @@
 import React from 'react';
 import PropTypes from 'prop-types';
 import classNames from 'classnames';
-import resolveUri from '../lib/resolve-uri';
+import resolveBadgeUrl from '../lib/badge-url';
 
 const Badge = ({ title, previewUri, exampleUri, documentation, baseUri, longCache, onClick }) => {
   const handleClick = onClick ?
@@ -12,10 +12,10 @@ const Badge = ({ title, previewUri, exampleUri, documentation, baseUri, longCach
     ? (<img
       className={classNames('badge-img', { clickable: onClick })}
       onClick={handleClick}
-      src={resolveUri(previewUri, baseUri, { longCache } )}
+      src={resolveBadgeUrl(previewUri, baseUri, { longCache } )}
       alt="" />
     ) : '\u00a0'; // non-breaking space
-  const resolvedExampleUri = resolveUri(
+  const resolvedExampleUri = resolveBadgeUrl(
     exampleUri || previewUri,
     baseUri,
     { longCache: false });
@@ -106,5 +106,4 @@ BadgeExamples.propTypes = {
 module.exports = {
   Badge,
   BadgeExamples,
-  resolveUri,
 };

frontend/components/dynamic-badge-maker.js

@@ -1,5 +1,6 @@
 import React from 'react';
 import PropTypes from 'prop-types';
+import resolveBadgeUrl from '../lib/badge-url';
 
 export default class DynamicBadgeMaker extends React.Component {
   static propTypes = {
@@ -10,29 +11,30 @@ export default class DynamicBadgeMaker extends React.Component {
     type: 'json',
     label: '',
     uri: '',
-    colorB: '',
+    color: '',
     prefix: '',
     suffix: '',
     query: '',
   };
 
   makeBadgeUri () {
-    const result = new URL(
+    const { label, uri, color, query, prefix, suffix } = this.state;
+    const queryParams = {
+      label,
+      uri,
+      colorB: color,
+      query,
+    }
+    if (prefix) {
+      queryParams.prefix = prefix;
+    }
+    if (suffix) {
+      queryParams.suffix = suffix;
+    }
+    return resolveBadgeUrl(
       `/dynamic/${this.state.type}.svg`,
-      this.props.baseUri || document.location.href);
-
-    const searchParams = [
-      'label',
-      'uri',
-      'colorB',
-      'prefix',
-      'suffix',
-      'query',
-    ];
-    searchParams.forEach(k => {
-      result.searchParams.set(k, this.state[k]);
-    });
-    return result.href;
+      this.props.baseUri || document.location.href,
+      { queryParams });
   }
 
   handleSubmit(e) {

frontend/components/footer.js

@@ -1,6 +1,6 @@
 import React from 'react';
 import PropTypes from 'prop-types';
-import resolveUri from '../lib/resolve-uri';
+import resolveUrl from '../lib/resolve-url';
 
 const Footer = ({ baseUri }) => (
   <section>
@@ -12,7 +12,7 @@ const Footer = ({ baseUri }) => (
     </p>
     <p>
       <object
-        data={resolveUri('/twitter/follow/shields_io.svg?style=social&label=Follow', baseUri)}
+        data={resolveUrl('/twitter/follow/shields_io.svg?style=social&label=Follow', baseUri)}
         alt="Follow @shields_io" />
       <a href="https://opencollective.com/shields" alt="Donate to us!">
         <img src="https://opencollective.com/shields/backers/badge.svg" />
@@ -21,10 +21,10 @@ const Footer = ({ baseUri }) => (
         <img src="https://opencollective.com/shields/sponsors/badge.svg" />
       </a>
       <object
-        data={resolveUri('/github/forks/badges/shields.svg?style=social&label=Fork', baseUri)}
+        data={resolveUrl('/github/forks/badges/shields.svg?style=social&label=Fork', baseUri)}
         alt="Fork on GitHub" />
       <object
-        data={resolveUri('/discord/308323056592486420.svg?style=social&label=Chat&link=https://discord.gg/HjJCwm5', baseUri)}
+        data={resolveUrl('/discord/308323056592486420.svg?style=social&label=Chat&link=https://discord.gg/HjJCwm5', baseUri)}
         alt="chat on Discord" />
     </p>
     <p>

frontend/components/markup-modal.js

@@ -2,7 +2,7 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import Modal from 'react-modal';
 import ClickToSelect from '@mapbox/react-click-to-select';
-import { resolveUri } from './badge-examples';
+import resolveBadgeUrl from '../lib/badge-url';
 import generateAllMarkup from '../lib/generate-image-markup';
 
 export default class MarkupModal extends React.Component {
@@ -39,7 +39,7 @@ export default class MarkupModal extends React.Component {
     // user.
     const { exampleUri, previewUri, link } = example;
     this.setState({
-      badgeUri: resolveUri(exampleUri || previewUri, baseUri || window.location.href),
+      badgeUri: resolveBadgeUrl(exampleUri || previewUri, baseUri || window.location.href),
       link,
     });
   }
@@ -53,10 +53,11 @@ export default class MarkupModal extends React.Component {
     const { title } = this.props.example;
     const { badgeUri, link, style } = this.state;
 
-    const withStyle = new URL(badgeUri, baseUri || window.location.href);
-    if (style !== 'flat') { // Default style doesn't need to be specified.
-      withStyle.searchParams.set('style', style);
-    }
+    const withStyle = resolveBadgeUrl(
+      badgeUri,
+      baseUri || window.location.href,
+      // Default style doesn't need to be specified.
+      style === 'flat' ? undefined : { style });
 
     return generateAllMarkup(withStyle.href, link, title);
   }

frontend/components/static-badge-maker.js

@@ -1,6 +1,6 @@
 import React from 'react';
 import PropTypes from 'prop-types';
-import staticBadgeUri from '../lib/static-badge-uri';
+import { staticBadgeUrl } from '../lib/badge-url';
 
 export default class StaticBadgeMaker extends React.Component {
   static propTypes = {
@@ -18,7 +18,7 @@ export default class StaticBadgeMaker extends React.Component {
 
     const { baseUri } = this.props;
     const { subject, status, color } = this.state;
-    const badgeUri = staticBadgeUri(baseUri || window.location.href, subject, status, color);
+    const badgeUri = staticBadgeUrl(baseUri || window.location.href, subject, status, color);
 
     document.location = badgeUri;
   }

frontend/components/suggestion-and-search.js

@@ -3,6 +3,7 @@ import PropTypes from 'prop-types';
 import fetchPonyfill from 'fetch-ponyfill';
 import debounce from 'lodash.debounce';
 import { Badge } from './badge-examples';
+import resolveUrl from '../lib/resolve-url';
 
 export default class SuggestionAndSearch extends React.Component {
   static propTypes = {
@@ -39,8 +40,7 @@ export default class SuggestionAndSearch extends React.Component {
       const { baseUri } = this.props;
       const { projectUri } = this.state;
 
-      const url = new URL('/$suggest/v1', baseUri);
-      url.searchParams.set('url', projectUri);
+      const url = resolveUrl('/$suggest/v1', baseUri, { url: projectUri });
 
       const fetch = window.fetch || fetchPonyfill;
       fetch(url)

frontend/components/usage.js

@@ -2,15 +2,16 @@ import { Fragment, default as React } from 'react';
 import PropTypes from 'prop-types';
 import StaticBadgeMaker from './static-badge-maker';
 import DynamicBadgeMaker from './dynamic-badge-maker';
-import staticBadgeUri from '../lib/static-badge-uri';
+import { staticBadgeUrl } from '../lib/badge-url';
 
 export default class Usage extends React.PureComponent {
   static propTypes = {
     baseUri: PropTypes.string.isRequired,
+    longCache: PropTypes.bool.isRequired,
   };
 
   renderColorExamples () {
-    const { baseUri } = this.props;
+    const { baseUri, longCache } = this.props;
     const colors = [
       'brightgreen',
       'green',
@@ -28,7 +29,7 @@ export default class Usage extends React.PureComponent {
           <Fragment key={i}>
             <img
               className="badge-img"
-              src={staticBadgeUri(baseUri, 'color', color, color)}
+              src={staticBadgeUrl(baseUri, 'color', color, color, { longCache })}
               alt={color} /> {}
           </Fragment>
         ))}
@@ -37,7 +38,7 @@ export default class Usage extends React.PureComponent {
   }
 
   renderStyleExamples () {
-    const { baseUri } = this.props;
+    const { baseUri, longCache } = this.props;
     const styles = [
       'plastic',
       'flat',
@@ -49,7 +50,12 @@ export default class Usage extends React.PureComponent {
       <table className="badge-img">
         <tbody>
           { styles.map((style, i) => {
-            const badgeUri = staticBadgeUri(baseUri, 'style', style, 'green', { style });
+            const badgeUri = staticBadgeUrl(
+              baseUri,
+              'style',
+              style,
+              'green',
+              { longCache, style });
             return (
               <tr key={i}>
                 <td>

frontend/lib/badge-url.js

@@ -0,0 +1,22 @@
+import resolveUrl from './resolve-url';
+
+export default function resolveBadgeUrl(url, baseUrl, options) {
+  const { longCache, style, queryParams: inQueryParams } = options || {};
+  const outQueryParams = Object.assign({}, inQueryParams);
+  if (longCache) {
+    outQueryParams.maxAge = '2592000';
+  }
+  if (style) {
+    outQueryParams.style = style;
+  }
+  return resolveUrl(url, baseUrl, outQueryParams);
+}
+
+export function encodeField(s) {
+  return encodeURIComponent(s.replace(/-/g, '--').replace(/_/g, '__'));
+}
+
+export function staticBadgeUrl(baseUrl, subject, status, color, options) {
+  const path = [subject, status, color].map(encodeField).join('-');
+  return resolveUrl(`/badge/${path}.svg`, baseUrl, options);
+}

frontend/lib/badge-url.spec.js

@@ -0,0 +1,34 @@
+import { test, given } from 'sazerac';
+import { default as resolveBadgeUrl, encodeField, staticBadgeUrl } from './badge-url';
+
+const resolveBadgeUrlWithLongCache = (url, baseUrl) =>
+  resolveBadgeUrl(url, baseUrl, { longCache: true })
+
+describe('Badge URL functions', function() {
+  test(resolveBadgeUrl, () => {
+    given('/badge/foo-bar-blue.svg', undefined)
+      .expect('/badge/foo-bar-blue.svg');
+    given('/badge/foo-bar-blue.svg', 'http://example.com')
+      .expect('http://example.com/badge/foo-bar-blue.svg');
+  });
+
+  test(resolveBadgeUrlWithLongCache, () => {
+    given('/badge/foo-bar-blue.svg', undefined)
+      .expect('/badge/foo-bar-blue.svg?maxAge=2592000');
+    given('/badge/foo-bar-blue.svg', 'http://example.com')
+      .expect('http://example.com/badge/foo-bar-blue.svg?maxAge=2592000');
+  })
+
+  test(encodeField, () => {
+    given('foo').expect('foo');
+    given('').expect('');
+    given('happy go lucky').expect('happy%20go%20lucky');
+    given('do-right').expect('do--right');
+    given('it_is_a_snake').expect('it__is__a__snake');
+  });
+
+  test(staticBadgeUrl, () => {
+    given('http://img.example.com', 'foo', 'bar', 'blue', { style: 'plastic'})
+      .expect('http://img.example.com/badge/foo-bar-blue.svg?style=plastic');
+  });
+});

frontend/lib/resolve-url.js

@@ -0,0 +1,14 @@
+// I played with build-url and url-resolve-browser and neither of them did the
+// right thing. Previously this was based on url-path, which patched around
+// the URL API. This caused problems in Firefox 57, but only in the production
+// build.
+import { resolve, parse, format } from 'url';
+
+// baseUrl and queryParams are optional.
+export default function resolveUrl (url, baseUrl, queryParams) {
+  const resolved = baseUrl ? resolve(baseUrl, url) : url;
+  const parsed = parse(resolved, /* parseQueryString */ true);
+  parsed.query = Object.assign({}, parsed.query, queryParams);
+  delete parsed.search;
+  return format(parsed);
+}

frontend/lib/resolve-url.spec.js

@@ -0,0 +1,29 @@
+import { test, given, forCases } from 'sazerac';
+import resolveUrl from './resolve-url';
+
+describe('URL resolver', function() {
+  test(resolveUrl, () => {
+    forCases([
+      given('/foo/bar'),
+      given('/foo/bar', '/'),
+      given('/foo/bar', '/baz'),
+      given('/foo/bar', '/baz/'),
+      given('/foo/bar', ''),
+      given('/foo/bar', undefined),
+    ]).expect('/foo/bar')
+
+    given('foo/bar', '/baz/').expect('/baz/foo/bar')
+
+    forCases([
+      given('http://foo/bar'),
+      given('bar', 'http://foo/'),
+      given('/bar', 'http://foo/'),
+    ]).expect('http://foo/bar')
+
+    given('/foo/bar', '/baz', { baz: 'bazinga' })
+      .expect('/foo/bar?baz=bazinga');
+
+    given('/foo/bar?thing=1', undefined, { other: '2' })
+      .expect('/foo/bar?thing=1&other=2');
+  })
+});