Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cherry pick style-spec build fixes from master #6603

Merged
merged 5 commits into from May 3, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions .gitignore
@@ -1,7 +1,6 @@
/rollup/build/
/docs/components/api.json
/dist/mapbox-gl-dev.js
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this change tied to #6513?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I duplicated it here because the style-spec build changes also introduce new stuff into dist/

/dist/mapbox-gl.js
/dist/
/docs/pages/dist/mapbox-gl-dev.js
/docs/pages/dist/mapbox-gl.js
*.js.map
Expand Down
2 changes: 1 addition & 1 deletion build/rollup_plugins.js
Expand Up @@ -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) => ({
Expand Down
2 changes: 2 additions & 0 deletions circle.yml
Expand Up @@ -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:
Expand Down
8 changes: 5 additions & 3 deletions package.json
Expand Up @@ -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",
Expand All @@ -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": [
Expand Down
10 changes: 4 additions & 6 deletions 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",
Expand All @@ -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",
Expand All @@ -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"
Expand Down
21 changes: 18 additions & 3 deletions 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: {
Expand All @@ -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;
41 changes: 21 additions & 20 deletions src/style-spec/style-spec.js
Expand Up @@ -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';
Expand All @@ -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;
82 changes: 38 additions & 44 deletions test/build/style-spec.test.js
Expand Up @@ -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;
Expand All @@ -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();
});
4 changes: 3 additions & 1 deletion 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) => {
Expand Down
4 changes: 3 additions & 1 deletion 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) => {
Expand Down