Cherry pick style-spec build fixes from master (#6603)

* Fix style-spec build (#6575) * Ensure style-spec is published with mapbox-gl (#6586) Cherry pick 478dd56 from master * Ensure style spec is published with build Closes #6433 Closes #6574 * Consolidate build unit test commands * Workaround CI image path bash/sh issue mapbox/mbgl-ci-images#17 * Bump style-spec version to 12.0.0-pre.1 (#6595) * Use named exports in style-spec.js (#6602) Cherry pick e83054f from master Fixes #6601 * Ignore whole /dist/ directory Done in master with e83054f
mapbox · May 3, 2018 · 3bd1803 · 3bd1803
1 parent 6a457ed
commit 3bd1803
Show file tree

Hide file tree

Showing 10 changed files with 96 additions and 81 deletions.
diff --git a/.gitignore b/.gitignore
@@ -1,7 +1,6 @@
 /rollup/build/
 /docs/components/api.json
-/dist/mapbox-gl-dev.js
-/dist/mapbox-gl.js
+/dist/
 /docs/pages/dist/mapbox-gl-dev.js
 /docs/pages/dist/mapbox-gl.js
 *.js.map

diff --git a/build/rollup_plugins.js b/build/rollup_plugins.js
@@ -38,7 +38,7 @@ export const plugins = () => [
 
 // Using this instead of rollup-plugin-flow due to
 // https://github.com/leebyron/rollup-plugin-flow/issues/5
-function flow() {
+export function flow() {
     return {
         name: 'flow-remove-types',
         transform: (code) => ({

diff --git a/circle.yml b/circle.yml
@@ -130,6 +130,8 @@ jobs:
           at: .
       - run: yarn run build-min
       - run: yarn run build-dev
+      - run: yarn run build-style-spec
+      - run: yarn run test-build
       - persist_to_workspace:
           root: .
           paths:

diff --git a/package.json b/package.json
@@ -114,9 +114,10 @@
     "sourceCache": true
   },
   "scripts": {
-    "build-dev": "rollup -c --environment BUILD:dev && build/run-tap --no-coverage test/build/dev.test.js",
+    "build-dev": "rollup -c --environment BUILD:dev",
     "watch-dev": "rollup -c --environment BUILD:dev --watch",
-    "build-min": "rollup -c --environment BUILD:production && build/run-tap --no-coverage test/build/min.test.js",
+    "build-min": "rollup -c --environment BUILD:production",
+    "build-style-spec": "cd src/style-spec && npm run build && cd ../.. && mkdir -p dist/style-spec && cp src/style-spec/dist/* dist/style-spec",
     "build-token": "node build/generate-access-token-script.js",
     "build-benchmarks": "BENCHMARK_VERSION=${BENCHMARK_VERSION:-\"$(git rev-parse --abbrev-ref HEAD) $(git rev-parse --short=7 HEAD)\"} rollup -c bench/rollup_config_benchmarks.js",
     "build-benchmarks-view": "BENCHMARK_VERSION=${BENCHMARK_VERSION:-\"$(git rev-parse --abbrev-ref HEAD) $(git rev-parse --short=7 HEAD)\"} rollup -c bench/rollup_config_benchmarks_view.js",
@@ -137,13 +138,14 @@
     "test-suite": "run-s test-render test-query",
     "test-suite-clean": "find test/integration/{render,query}-tests -mindepth 2 -type d  -not \\( -exec test -e \"{}/style.json\" \\; \\) -print | xargs -t rm -r",
     "test-unit": "build/run-tap --reporter classic --no-coverage test/unit",
+    "test-build": "build/run-tap --no-coverage test/build/**/*.test.js",
     "test-render": "node --max-old-space-size=2048 test/render.test.js",
     "test-query": "node test/query.test.js",
     "test-expressions": "build/run-node test/expression.test.js",
     "test-flow": "node build/generate-flow-typed-style-spec && flow .",
     "test-flow-cov": "flow-coverage-report -i 'src/**/*.js' -t html",
     "test-cov": "nyc --require=@mapbox/flow-remove-types/register --reporter=text-summary --reporter=lcov --cache run-s test-unit test-expressions test-query test-render",
-    "prepublish": "in-publish && run-s build-dev build-min || not-in-publish",
+    "prepublish": "in-publish && run-s build-dev build-min build-style-spec test-build || not-in-publish",
     "codegen": "build/run-node build/generate-style-code.js && build/run-node build/generate-struct-arrays.js"
   },
   "files": [

diff --git a/src/style-spec/package.json b/src/style-spec/package.json
@@ -1,7 +1,7 @@
 {
   "name": "@mapbox/mapbox-gl-style-spec",
   "description": "a specification for mapbox gl styles",
-  "version": "11.1.1",
+  "version": "12.0.0-pre.1",
   "author": "Mapbox",
   "keywords": [
     "mapbox",
@@ -12,9 +12,9 @@
   "main": "./dist/index.js",
   "scripts": {
     "copy-flow-typed": "cp -R ../../flow-typed .",
-    "build": "rollup -c",
-    "prepublish": "in-publish && yarn copy-flow-typed && yarn build || not-in-publish",
-    "postpublish": "in-publish && rm -r flow-typed dist/index.js || not-in-publish"
+    "build": "../../node_modules/.bin/rollup -c",
+    "prepublish": "yarn copy-flow-typed && yarn build",
+    "postpublish": "rm -r flow-typed dist/index.js"
   },
   "repository": {
     "type": "git",
@@ -29,9 +29,7 @@
   "dependencies": {
     "@mapbox/jsonlint-lines-primitives": "~2.0.1",
     "@mapbox/unitbezier": "^0.0.0",
-    "brfs": "^1.4.0",
     "csscolorparser": "~1.0.2",
-    "in-publish": "^2.0.0",
     "minimist": "0.0.8",
     "rw": "^1.3.3",
     "sort-object": "^0.3.2"

diff --git a/src/style-spec/rollup.config.js b/src/style-spec/rollup.config.js
@@ -1,5 +1,11 @@
 import replace from 'rollup-plugin-replace';
-import {plugins} from '../../build/rollup_plugins';
+import buble from 'rollup-plugin-buble';
+import resolve from 'rollup-plugin-node-resolve';
+import commonjs from 'rollup-plugin-commonjs';
+import unassert from 'rollup-plugin-unassert';
+import json from 'rollup-plugin-json';
+import {flow} from '../../build/rollup_plugins';
+
 const config = [{
     input: `${__dirname}/style-spec.js`,
     output: {
@@ -16,7 +22,16 @@ const config = [{
             values: {
                 '_token_stack:': ''
             }
-        })
-    ].concat(plugins())
+        }),
+        flow(),
+        json(),
+        buble({transforms: {dangerousForOf: true}, objectAssign: "Object.assign"}),
+        unassert(),
+        resolve({
+            browser: true,
+            preferBuiltins: false
+        }),
+        commonjs()
+    ]
 }];
 export default config;
diff --git a/src/style-spec/style-spec.js b/src/style-spec/style-spec.js
@@ -51,8 +51,6 @@ export type StylePropertySpecification = {
 };
 
 import v8 from './reference/v8.json';
-export {v8};
-
 import latest from './reference/latest';
 import format from './format';
 import migrate from './migrate';
@@ -68,35 +66,38 @@ import convertFunction from './function/convert';
 
 import validate from './validate_style';
 
-const exported = {
+const expression = {
+    StyleExpression,
+    isExpression,
+    createExpression,
+    createPropertyExpression,
+    normalizePropertyExpression,
+    ZoomConstantExpression,
+    ZoomDependentExpression,
+    StylePropertyFunction
+};
+
+const styleFunction = {
+    convertFunction,
+    createFunction,
+    isFunction
+};
+
+export {
+    v8,
     latest,
     format,
     migrate,
     composite,
     diff,
     ValidationError,
     ParsingError,
-    expression: {
-        StyleExpression,
-        isExpression,
-        createExpression,
-        createPropertyExpression,
-        normalizePropertyExpression,
-        ZoomConstantExpression,
-        ZoomDependentExpression,
-        StylePropertyFunction
-    },
+    expression,
     featureFilter,
     Color,
-    function: {
-        convertFunction,
-        createFunction,
-        isFunction
-    },
+    styleFunction as function,
     validate
 };
 
-export default exported;
-
 validate.parsed = validate;
 validate.latest = validate;
diff --git a/test/build/style-spec.test.js b/test/build/style-spec.test.js
@@ -4,7 +4,6 @@
 const test = require('mapbox-gl-js-test').test;
 const fs = require('fs');
 const path = require('path');
-const exec = require('child_process').exec;
 const isBuiltin = require('is-builtin-module');
 
 const Linter = require('eslint').Linter;
@@ -15,57 +14,52 @@ import rollupConfig from '../../src/style-spec/rollup.config';
 // some paths
 const styleSpecDirectory = path.join(__dirname, '../../src/style-spec');
 const styleSpecPackage = require('../../src/style-spec/package.json');
-const styleSpecDistBundle = path.join(styleSpecDirectory, styleSpecPackage.main);
+const styleSpecDistBundle = fs.readFileSync(path.join(__dirname, '../../dist/style-spec/index.js'), 'utf-8');
 
 test('@mapbox/mapbox-gl-style-spec npm package', (t) => {
-    // simulate npm install of @mapbox/mapbox-gl-style-spec
     t.test('build plain ES5 bundle in prepublish', (t) => {
-        t.tearDown(() => {
-            fs.unlink(styleSpecDistBundle, (error) => {
-                if (error) console.error(error);
-            });
-        });
+        const linter = new Linter();
+        const messages = linter.verify(styleSpecDistBundle, {
+            parserOptions: {
+                ecmaVersion: 5
+            },
+            rules: {},
+            env: {
+                node: true
+            }
+        }).map(message => `${message.line}:${message.column}: ${message.message}`);
+        t.deepEqual(messages, [], 'distributed bundle is plain ES5 code');
 
-        exec(`rm -f ${styleSpecDistBundle} && npm run build`, { cwd: styleSpecDirectory }, (error) => {
-            t.error(error);
-            t.ok(fs.existsSync(styleSpecDistBundle), 'dist bundle exists');
+        t.stub(console, 'warn');
+        rollup.rollup({
+            input: `${styleSpecDirectory}/style-spec.js`,
+            plugins: [{
+                resolveId: (id, importer) => {
+                    if (
+                        /^[\/\.]/.test(id) ||
+                        isBuiltin(id) ||
+                        /node_modules/.test(importer)
+                    ) {
+                        return null;
+                    }
 
-            const linter = new Linter();
-            const messages = linter.verify(fs.readFileSync(styleSpecDistBundle, 'utf-8'), {
-                parserOptions: {
-                    ecmaVersion: 5
-                },
-                rules: {},
-                env: {
-                    node: true
+                    t.ok(styleSpecPackage.dependencies[id], `External dependency ${id} (imported from ${importer}) declared in style-spec's package.json`);
+                    return false;
                 }
-            }).map(message => `${message.line}:${message.column}: ${message.message}`);
-            t.deepEqual(messages, [], 'distributed bundle is plain ES5 code');
-
-            t.stub(console, 'warn');
-            rollup.rollup({
-                input: `${styleSpecDirectory}/style-spec.js`,
-                plugins: [{
-                    resolveId: (id, importer) => {
-                        if (
-                            /^[\/\.]/.test(id) ||
-                            isBuiltin(id) ||
-                            /node_modules/.test(importer)
-                        ) {
-                            return null;
-                        }
-
-                        t.ok(styleSpecPackage.dependencies[id], `External dependency ${id} (imported from ${importer}) declared in style-spec's package.json`);
-                        return false;
-                    }
-                }].concat(rollupConfig[0].plugins)
-            }).then(() => {
-                t.end();
-            }).catch(e => {
-                t.error(e);
-            });
+            }].concat(rollupConfig[0].plugins)
+        }).then(() => {
+            t.end();
+        }).catch(e => {
+            t.error(e);
         });
     });
 
+    t.test('exports components directly, not behind `default` - https://github.com/mapbox/mapbox-gl-js/issues/6601', (t) => {
+        const spec = require('../../dist/style-spec/index.js');
+        t.ok(spec.validate);
+        t.notOk(spec.default && spec.default.validate);
+        t.end();
+    });
+
     t.end();
 });
diff --git a/test/unit/style-spec/migrate.test.js b/test/unit/style-spec/migrate.test.js
@@ -1,12 +1,14 @@
 import { test as t } from 'mapbox-gl-js-test';
 import fs from 'fs';
 import glob from 'glob';
-import spec from '../../../src/style-spec/style-spec';
 import path from 'path';
 import validate from '../../../src/style-spec/validate_style';
 import v8 from '../../../src/style-spec/reference/v8';
 import migrate from '../../../src/style-spec/migrate';
 
+/* eslint-disable import/namespace */
+import * as spec from '../../../src/style-spec/style-spec';
+
 const UPDATE = !!process.env.UPDATE;
 
 t('does not migrate from version 5', (t) => {

diff --git a/test/unit/style-spec/spec.test.js b/test/unit/style-spec/spec.test.js
@@ -1,5 +1,7 @@
 import { test } from 'mapbox-gl-js-test';
-import spec from '../../../src/style-spec/style-spec';
+
+/* eslint-disable import/namespace */
+import * as spec from '../../../src/style-spec/style-spec';
 
 ['v8', 'latest'].forEach((version) => {
     ['', 'min'].forEach((kind) => {