From 3bd1803b7d89a6a25b56b67ed6dd82414fb97200 Mon Sep 17 00:00:00 2001 From: Anand Thakker Date: Thu, 3 May 2018 12:51:16 -0400 Subject: [PATCH] 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 478dd560b1f91a11feb5fc814145106f9253941d 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 https://github.com/mapbox/mbgl-ci-images/issues/17 * Bump style-spec version to 12.0.0-pre.1 (#6595) * Use named exports in style-spec.js (#6602) Cherry pick e83054fa16f741b6b62517260e55fbc20baf16e8 from master Fixes #6601 * Ignore whole /dist/ directory Done in master with e83054fa16f741b6b62517260e55fbc20baf16e8 --- .gitignore | 3 +- build/rollup_plugins.js | 2 +- circle.yml | 2 + package.json | 8 ++- src/style-spec/package.json | 10 ++-- src/style-spec/rollup.config.js | 21 ++++++- src/style-spec/style-spec.js | 41 +++++++------- test/build/style-spec.test.js | 82 +++++++++++++--------------- test/unit/style-spec/migrate.test.js | 4 +- test/unit/style-spec/spec.test.js | 4 +- 10 files changed, 96 insertions(+), 81 deletions(-) diff --git a/.gitignore b/.gitignore index b4605b4fc7d..47652a61268 100644 --- 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 index ae7cf80e731..244e53f17aa 100644 --- 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 index 3149212e422..6da5ba69f50 100644 --- 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 index 7c4dc50d9b1..f55fa06990b 100644 --- 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 index 18f0a532e73..fbc96929f9f 100644 --- 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 index 09ed5c47f35..61db3eb1a29 100644 --- 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 index 51c6ac7b92c..4ffa6f9e8ef 100644 --- 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,7 +66,25 @@ 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, @@ -76,27 +92,12 @@ const exported = { 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 index f0df85c47fd..94793509d6a 100644 --- 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 index 1b93747cfbc..24eab4aa237 100644 --- 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 index 80751ba13de..ad58f701d28 100644 --- 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) => {