Merge pull request #121 from conveyal/dev

v3.4.0

README.md

@@ -17,6 +17,7 @@
   * [Deploy](#deploy)
   * [Lint](#lint)
   * [Test](#test)
+  * [Lint Messages](#lint-messages)
 
 ## Install
 
@@ -71,6 +72,7 @@ $ mastarm --help
     deploy [entries...] [options]  Bundle & Deploy JavaScript & CSS
     lint [paths...]                Lint JavaScript [& CSS coming soon!]
     test [options]                 Run tests using Jest test runner
+    lint-messages [paths...]       Lint message strings, making sure all messages used in source files are present in messages.yml
 
   Options:
 
@@ -171,6 +173,44 @@ Options:
 
 ```
 
+### `lint-messages`
+
+```shell
+$ mastarm lint-messages
+
+Usage: lint-messages [options] [paths...]
+
+Check that all messages used in source code are present in config. Pass in path to source file(s). Set the config with --config.
+
+```
+
+This checks to ensure that all of the messages referenced in JS code are defined in the `messages.yml`
+file. It defaults to using the messages in `configurations/default/messages.yml`, however a different
+config can be specified with `--config`. By default it will check the JS files in `lib`, but you can
+also pass in an arbitrary number of paths to directories or files to lint.
+
+`lint-messages` is somewhat opinionated about how messages should be used in code. They should be imported
+from a local module called `messages`, and referred to using dot notation. It will work regardless
+of whether you import the top-level messages object or named children; the following all work:
+
+    import messages from '../utils/messages'
+    import msgs from './messages'
+    import { analysis } from './messages'
+    import msgs, { project as proj } from '../messages'
+    import {analysis, project as proj}, msgs from '../messages'
+
+and permutations thereof. Messages should be referred to directly from these top-level imports, i.e.
+you should not refer to messages like this:
+
+    import messages from './messages'
+    const { analysis, project } = messages
+    return analysis.newScenario
+
+but the following is fine:
+
+    import { analysis } from './messages'
+    return analysis.newScenario
+
 [npm-image]: https://img.shields.io/npm/v/mastarm.svg?maxAge=2592000&style=flat-square
 [npm-url]: https://www.npmjs.com/package/mastarm
 [travis-image]: https://img.shields.io/travis/conveyal/mastarm.svg?style=flat-square

__tests__/bin/mastarm.js

@@ -40,7 +40,7 @@ describe('mastarm cli', () => {
       exec(`node ${mastarm} build ${mockDir}/index.js:${buildDir}/index.js ${mockDir}/index.css:${buildDir}/index.css`,
         (err, stdout, stderr) => {
           expect(err).toBeNull()
-          expect(stdout).toBe('')
+          expect(stdout).toContain('updated css file')
           expect(stderr).toBe('')
           expect(fs.existsSync(`${buildDir}/index.js`)).toBeTruthy()
           expect(fs.existsSync(`${buildDir}/index.css`)).toBeTruthy()

__tests__/lib/__snapshots__/lint-messages.js.snap

@@ -0,0 +1,103 @@
+exports[`import { analysis, project as proj } from '../utils/messages' 1`] = `
+Array [
+  undefined,
+  Array [
+    Array [
+      "analysis",
+      "analysis",
+    ],
+    Array [
+      "project",
+      "proj",
+    ],
+  ],
+]
+`;
+
+exports[`import { analysis, project as proj }, messages from '../utils/messages' 1`] = `
+Array [
+  "messages",
+  Array [
+    Array [
+      "analysis",
+      "analysis",
+    ],
+    Array [
+      "project",
+      "proj",
+    ],
+  ],
+]
+`;
+
+exports[`import {analysis, project as proj} from '../utils/messages' 1`] = `
+Array [
+  undefined,
+  Array [
+    Array [
+      "analysis",
+      "analysis",
+    ],
+    Array [
+      "project",
+      "proj",
+    ],
+  ],
+]
+`;
+
+exports[`import messages, { analysis, project as proj } from '../utils/messages' 1`] = `
+Array [
+  "messages",
+  Array [
+    Array [
+      "analysis",
+      "analysis",
+    ],
+    Array [
+      "project",
+      "proj",
+    ],
+  ],
+]
+`;
+
+exports[`import msgs from '../utils/messages' 1`] = `
+Array [
+  "msgs",
+  Array [],
+]
+`;
+
+exports[`lint-messages should lint file 1`] = `
+Array [
+  Array [
+    "common.downloadRegional",
+    6,
+  ],
+  Array [
+    "analysis.stop",
+    8,
+  ],
+  Array [
+    "analysis.stop",
+    9,
+  ],
+  Array [
+    "project.delete",
+    12,
+  ],
+  Array [
+    "project.delete",
+    13,
+  ],
+  Array [
+    "common.runRegional",
+    14,
+  ],
+  Array [
+    "common.stopRegional",
+    14,
+  ],
+]
+`;

__tests__/lib/lint-messages.js

@@ -0,0 +1,60 @@
+/* globals describe, expect, it */
+
+const testFile = `
+import messages, { analysis, project as proj } from '../utils/messages'
+import Component, { React, PropTypes } from 'react'
+
+const test = messages.common.download
+const test2 = messages.common.downloadRegional
+const test3 = <div>{analysis.run}</div>
+const test3b = <div>{analysis.stop}</div>
+const test4 = analysis.stop
+const test5 = proj.new
+// handle multiple messages in one line
+const test6 = test5 ? proj.delete : proj.new
+const test7 = test6 ? proj.new : proj.delete
+const test8 = test7 ? messages.common.runRegional : messages.common.stopRegional
+`
+
+// test messages, some of the referenced messages are undefined
+const testMessages = {
+  common: {
+    download: 'Download'
+  },
+  analysis: {
+    run: 'Run'
+  },
+  project: {
+    new: 'New'
+  }
+}
+
+describe('lint-messages', () => {
+  const { lintFileContents, parseImportLine } = require('../../lib/lint-messages')
+  it('should parse root import correctly', () => {
+    expect(parseImportLine("import msgs from '../utils/messages'"))
+      .toMatchSnapshot("import msgs from '../utils/messages'")
+  })
+
+  it('should parse named imports correctly', () => {
+    expect(parseImportLine("import { analysis, project as proj } from '../utils/messages'"))
+      .toMatchSnapshot("import { analysis, project as proj } from '../utils/messages'")
+
+    // make sure it works when spacing is smaller
+    expect(parseImportLine("import {analysis, project as proj} from '../utils/messages'"))
+      .toMatchSnapshot("import {analysis, project as proj} from '../utils/messages'")
+  })
+
+  it('should parse named and default imports together correctly', () => {
+    expect(parseImportLine("import messages, { analysis, project as proj } from '../utils/messages'"))
+      .toMatchSnapshot("import messages, { analysis, project as proj } from '../utils/messages'")
+
+    // try it with the default import after the named imports
+    expect(parseImportLine("import { analysis, project as proj }, messages from '../utils/messages'"))
+      .toMatchSnapshot("import { analysis, project as proj }, messages from '../utils/messages'")
+  })
+
+  it('should lint file', () => {
+    expect(lintFileContents(testMessages, testFile)).toMatchSnapshot()
+  })
+})

bin/mastarm

@@ -101,6 +101,27 @@ commander
     engine.cli(standardOptions)
   })
 
+commander
+  .command('lint-messages [paths...]')
+  .description('Check that all messages used in source code are present in config. Pass in path to source file(s). Set the config with --config.')
+  .action(function (paths) {
+    const lintMessages = require('../lib/lint-messages').lint
+    const config = loadConfig(process.cwd(), commander.config, commander.env)
+    // default to linting lib
+    const errors = lintMessages(paths.length > 0 ? paths : ['lib'], config.messages)
+
+    if (errors.length > 0) {
+      console.log(`${errors.length} missing messages`)
+      for (const [message, file, line] of errors) {
+        console.log(`${file} line ${line}: ${message} is not defined`)
+      }
+
+      process.exit(1)
+    } else {
+      console.log('No missing messages found! 💃')
+    }
+  })
+
 commander
   .command('prepublish [entries...]')
   .description('Transpile JavaScript down to ES5 with Babel')

lib/budo.js

@@ -28,21 +28,8 @@ module.exports = function ({
     pushstate: true
   }
   if (proxy) {
-    const httpProxy = require('http-proxy')
-    const proxyServer = httpProxy.createProxyServer({target: proxy})
-    proxyServer.on('error', function (err, req, res) {
-      console.error(err.stack)
-      res.writeHead(500, {'Content-Type': 'text/plain'})
-      res.end(err.message)
-    })
-    budoOpts.middleware.push(function (req, res, next) {
-      if (req.url.indexOf('/api') === 0) {
-        req.url = req.url.slice(4)
-        proxyServer.web(req, res)
-      } else {
-        next()
-      }
-    })
+    const middlewareProxy = require('middleware-proxy')
+    budoOpts.middleware.push(middlewareProxy('/api', proxy))
   }
   if (flyle) {
     const serveTiles = require('./flyle')

lib/build.js

@@ -53,6 +53,7 @@ function buildJs ({config, entry, env, minify, outfile, watch}) {
     pipeline.plugin(require('watchify'), {poll: true})
     pipeline.plugin(require('errorify'))
     pipeline.on('update', bundle)
+    pipeline.on('log', console.log)
   }
 
   return bundle()

lib/css-transform.js

@@ -42,6 +42,7 @@ module.exports = function ({
           if (results.map) {
             fs.writeFile(`${outfile}.map`, results.map, handleErr)
           }
+          console.log(`updated css file: ${outfile}`)
         }
         return results
       })