Merge pull request #6770 from tschaub/no-missing-requires

Enforce the "no missing requires" rule

.eslintignore

@@ -1,2 +1,3 @@
 examples/Jugl.js
 examples/resources/
+build/package/**/*webgl*

Makefile

@@ -86,7 +86,7 @@ check-deps:
 	done ;\
 
 .PHONY: ci
-ci: lint build test test-rendering compile-examples check-examples apidoc
+ci: lint build test test-rendering package compile-examples check-examples apidoc
 
 .PHONY: compile-examples
 compile-examples: build/compiled-examples/all.combined.js
@@ -253,7 +253,7 @@ build/timestamps/eslint-timestamp: $(SRC_JS) $(SPEC_JS) $(SPEC_RENDERING_JS) \
                                    build/timestamps/node-modules-timestamp
 	@mkdir -p $(@D)
 	@echo "Running eslint..."
-	@./node_modules/.bin/eslint --quiet tasks test test_rendering src examples
+	@./node_modules/.bin/eslint tasks test test_rendering src examples
 	@touch $@
 
 build/timestamps/node-modules-timestamp: package.json
@@ -310,3 +310,4 @@ package:
 	@rm build/package/typedefs.js
 	@cp css/ol.css build/package
 	./node_modules/.bin/jscodeshift --transform transforms/module.js build/package
+	npm run lint-package

package.json

@@ -13,6 +13,7 @@
     "postinstall": "closure-util update",
     "start": "node tasks/serve.js",
     "pretest": "eslint tasks test test_rendering src examples",
+    "lint-package": "eslint --fix build/package",
     "test": "node tasks/test.js",
     "debug-server": "node tasks/serve-lib.js"
   },
@@ -31,7 +32,6 @@
   ],
   "dependencies": {
     "async": "2.4.0",
-    "browserify": "14.3.0",
     "closure-util": "1.19.0",
     "derequire": "2.0.6",
     "fs-extra": "3.0.0",
@@ -45,8 +45,12 @@
     "pbf": "3.0.5",
     "pixelworks": "1.1.0",
     "rbush": "2.0.1",
+    "rollup": "^0.41.6",
+    "rollup-plugin-cleanup": "^1.0.0",
+    "rollup-plugin-commonjs": "^8.0.2",
+    "rollup-plugin-node-resolve": "^3.0.0",
     "temp": "0.8.3",
-    "vector-tile": "1.3.0",
+    "@mapbox/vector-tile": "1.3.0",
     "walk": "2.3.9"
   },
   "devDependencies": {
@@ -91,7 +95,7 @@
       "no-constant-condition": 0,
       "openlayers-internal/enum": 2,
       "openlayers-internal/no-duplicate-requires": 2,
-      "openlayers-internal/no-missing-requires": 1,
+      "openlayers-internal/no-missing-requires": 2,
       "openlayers-internal/no-unused-requires": 2,
       "openlayers-internal/one-provide": 2,
       "openlayers-internal/requires-first": 2,
@@ -101,21 +105,20 @@
   },
   "ext": [
     {
-      "module": "rbush",
-      "browserify": true
+      "module": "rbush"
     },
     {
       "module": "pbf",
-      "browserify": true
+      "name": "PBF"
     },
     {
       "module": "pixelworks",
-      "browserify": true
+      "import": "Processor"
     },
     {
-      "module": "vector-tile",
+      "module": "@mapbox/vector-tile",
       "name": "vectortile",
-      "browserify": true
+      "import": "VectorTile"
     }
   ]
 }

package/.eslintrc

@@ -0,0 +1,5 @@
+{
+  "parserOptions": {
+    "sourceType": "module"
+  }
+}

package/package.json

@@ -9,7 +9,7 @@
     "pbf": "3.0.5",
     "pixelworks": "1.1.0",
     "rbush": "2.0.1",
-    "vector-tile": "1.3.0"
+    "@mapbox/vector-tile": "1.3.0"
   },
   "browserify": {
     "transform": [

src/ol/format/mvt.js

@@ -3,8 +3,8 @@
 goog.provide('ol.format.MVT');
 
 goog.require('ol');
-goog.require('ol.ext.pbf');
-goog.require('ol.ext.vectortile');
+goog.require('ol.ext.PBF');
+goog.require('ol.ext.vectortile.VectorTile');
 goog.require('ol.format.Feature');
 goog.require('ol.format.FormatType');
 goog.require('ol.geom.GeometryLayout');
@@ -149,7 +149,7 @@ ol.format.MVT.prototype.readRenderFeature_ = function(rawFeature, layer) {
 ol.format.MVT.prototype.readFeatures = function(source, opt_options) {
   var layers = this.layers_;
 
-  var pbf = new ol.ext.pbf(/** @type {ArrayBuffer} */ (source));
+  var pbf = new ol.ext.PBF(/** @type {ArrayBuffer} */ (source));
   var tile = new ol.ext.vectortile.VectorTile(pbf);
   var features = [];
   var featureClass = this.featureClass_;

src/ol/render/webgl/circlereplay.js

@@ -204,8 +204,8 @@ if (ol.ENABLE_WEBGL) {
     // get the locations
     var locations;
     if (!this.defaultLocations_) {
-      locations =
-          new ol.render.webgl.circlereplay.defaultshader.Locations(gl, program);
+      // eslint-disable-next-line openlayers-internal/no-missing-requires
+      locations = new ol.render.webgl.circlereplay.defaultshader.Locations(gl, program);
       this.defaultLocations_ = locations;
     } else {
       locations = this.defaultLocations_;

src/ol/render/webgl/circlereplay/defaultshader.js

@@ -1,4 +1,5 @@
 // This file is automatically generated, do not edit
+/* eslint openlayers-internal/no-missing-requires: 0 */
 goog.provide('ol.render.webgl.circlereplay.defaultshader');
 
 goog.require('ol');

src/ol/render/webgl/imagereplay.js

@@ -377,8 +377,8 @@ if (ol.ENABLE_WEBGL) {
     // get the locations
     var locations;
     if (!this.defaultLocations_) {
-      locations =
-          new ol.render.webgl.imagereplay.defaultshader.Locations(gl, program);
+      // eslint-disable-next-line openlayers-internal/no-missing-requires
+      locations = new ol.render.webgl.imagereplay.defaultshader.Locations(gl, program);
       this.defaultLocations_ = locations;
     } else {
       locations = this.defaultLocations_;

src/ol/render/webgl/imagereplay/defaultshader.js

@@ -1,4 +1,5 @@
 // This file is automatically generated, do not edit
+/* eslint openlayers-internal/no-missing-requires: 0 */
 goog.provide('ol.render.webgl.imagereplay.defaultshader');
 
 goog.require('ol');

src/ol/render/webgl/linestringreplay.js

@@ -446,8 +446,8 @@ if (ol.ENABLE_WEBGL) {
     // get the locations
     var locations;
     if (!this.defaultLocations_) {
-      locations =
-          new ol.render.webgl.linestringreplay.defaultshader.Locations(gl, program);
+      // eslint-disable-next-line openlayers-internal/no-missing-requires
+      locations = new ol.render.webgl.linestringreplay.defaultshader.Locations(gl, program);
       this.defaultLocations_ = locations;
     } else {
       locations = this.defaultLocations_;

src/ol/render/webgl/linestringreplay/defaultshader.js

@@ -1,4 +1,5 @@
 // This file is automatically generated, do not edit
+/* eslint openlayers-internal/no-missing-requires: 0 */
 goog.provide('ol.render.webgl.linestringreplay.defaultshader');
 
 goog.require('ol');

src/ol/render/webgl/polygonreplay.js

@@ -833,8 +833,8 @@ if (ol.ENABLE_WEBGL) {
     // get the locations
     var locations;
     if (!this.defaultLocations_) {
-      locations =
-          new ol.render.webgl.polygonreplay.defaultshader.Locations(gl, program);
+      // eslint-disable-next-line openlayers-internal/no-missing-requires
+      locations = new ol.render.webgl.polygonreplay.defaultshader.Locations(gl, program);
       this.defaultLocations_ = locations;
     } else {
       locations = this.defaultLocations_;

src/ol/render/webgl/polygonreplay/defaultshader.js

@@ -1,4 +1,5 @@
 // This file is automatically generated, do not edit
+/* eslint openlayers-internal/no-missing-requires: 0 */
 goog.provide('ol.render.webgl.polygonreplay.defaultshader');
 
 goog.require('ol');

src/ol/renderer/webgl/defaultmapshader.js

@@ -1,4 +1,5 @@
 // This file is automatically generated, do not edit
+/* eslint openlayers-internal/no-missing-requires: 0 */
 goog.provide('ol.renderer.webgl.defaultmapshader');
 
 goog.require('ol');

src/ol/renderer/webgl/layer.js

@@ -156,8 +156,8 @@ if (ol.ENABLE_WEBGL) {
 
     var locations;
     if (!this.defaultLocations_) {
-      locations =
-          new ol.renderer.webgl.defaultmapshader.Locations(gl, program);
+      // eslint-disable-next-line openlayers-internal/no-missing-requires
+      locations = new ol.renderer.webgl.defaultmapshader.Locations(gl, program);
       this.defaultLocations_ = locations;
     } else {
       locations = this.defaultLocations_;

src/ol/renderer/webgl/tilelayer.js

@@ -196,8 +196,8 @@ if (ol.ENABLE_WEBGL) {
       var program = context.getProgram(this.fragmentShader_, this.vertexShader_);
       context.useProgram(program);
       if (!this.locations_) {
-        this.locations_ =
-            new ol.renderer.webgl.tilelayershader.Locations(gl, program);
+        // eslint-disable-next-line openlayers-internal/no-missing-requires
+        this.locations_ = new ol.renderer.webgl.tilelayershader.Locations(gl, program);
       }
 
       context.bindBuffer(ol.webgl.ARRAY_BUFFER, this.renderArrayBuffer_);

src/ol/renderer/webgl/tilelayershader.js

@@ -1,4 +1,5 @@
 // This file is automatically generated, do not edit
+/* eslint openlayers-internal/no-missing-requires: 0 */
 goog.provide('ol.renderer.webgl.tilelayershader');
 
 goog.require('ol');

src/ol/source/raster.js

@@ -7,7 +7,7 @@ goog.require('ol.dom');
 goog.require('ol.events');
 goog.require('ol.events.Event');
 goog.require('ol.events.EventType');
-goog.require('ol.ext.pixelworks');
+goog.require('ol.ext.pixelworks.Processor');
 goog.require('ol.extent');
 goog.require('ol.layer.Image');
 goog.require('ol.layer.Tile');

src/ol/webgl/shader.mustache

@@ -1,4 +1,5 @@
 // This file is automatically generated, do not edit
+/* eslint openlayers-internal/no-missing-requires: 0 */
 goog.provide('{{namespace}}');
 
 goog.require('ol');

tasks/.eslintrc

@@ -0,0 +1,6 @@
+{
+  "env": {
+    "node": true,
+    "es6": true
+  }
+}

tasks/build-ext.js

@@ -1,122 +1,69 @@
-var fs = require('fs-extra');
-var path = require('path');
-
-var async = require('async');
-var browserify = require('browserify');
-var derequire = require('derequire');
-
-var pkg = require('../package.json');
-
-var root = path.join(__dirname, '..');
-var buildDir = path.join(root, 'build', 'ol.ext');
-
+const cleanup = require('rollup-plugin-cleanup');
+const common = require('rollup-plugin-commonjs');
+const node = require('rollup-plugin-node-resolve');
+const path = require('path');
+const pkg = require('../package.json');
+const rollup = require('rollup').rollup;
 
 /**
- * Get external module metadata.
- * @return {Array.<Object>} Array of objects representing external modules.
+ * Wrap a bundled dependency for consumption by the Compiler.
+ * @param {Object} ext Details from the `ext` object in package.json.
+ * @return {Object} A rollup plugin.
  */
-function getExternalModules() {
-  return pkg.ext.map(function(item) {
-    if (typeof item === 'string') {
-      return {
-        name: item,
-        module: item,
-        main: require.resolve(item),
-        browserify: false
-      };
-    } else {
-      return {
-        module: item.module,
-        name: item.name !== undefined ? item.name : item.module,
-        main: require.resolve(item.module),
-        browserify: item.browserify !== undefined ? item.browserify : false
-      };
+function wrap(ext) {
+  return {
+    name: 'googup',
+    transformBundle: function(source) {
+      let name = `ol.ext.${ext.name || ext.module}`;
+      let postamble = '';
+      if (ext.import) {
+        name += '.' + ext.import;
+      } else {
+        postamble = `${name} = ${name}.default;\n`;
+      }
+      return `
+/**
+ * @fileoverview
+ * @suppress {accessControls, ambiguousFunctionDecl, checkDebuggerStatement, checkRegExp, checkTypes, checkVars, const, constantProperty, deprecated, duplicate, es5Strict, fileoverviewTags, missingProperties, nonStandardJsDocs, strictModuleDepCheck, suspiciousCode, undefinedNames, undefinedVars, unknownDefines, unusedLocalVariables, uselessCode, visibility}
+ */
+goog.provide('${name}');
+
+/** @typedef {function(*)} */
+${name} = function() {};
+
+(function() {${source}}).call(ol.ext);
+${postamble}`;
     }
-  });
-}
-
-
-/**
- * Wrap a CommonJS module in Closure Library accessible code.
- * @param {Object} mod Module metadata.
- * @param {function(Error, string)} callback Called with any error and the
- *     wrapped module.
- */
-function wrapModule(mod, callback) {
-  var wrap = function(code) {
-    return 'goog.provide(\'ol.ext.' + mod.name + '\');\n' +
-        '/** @typedef {function(*)} */\n' +
-        'ol.ext.' + mod.name + ';\n' +
-        '(function() {\n' +
-        'var exports = {};\n' +
-        'var module = {exports: exports};\n' +
-        'var define;\n' +
-        '/**\n' +
-        ' * @fileoverview\n' +
-        ' * @suppress {accessControls, ambiguousFunctionDecl, ' +
-        'checkDebuggerStatement, checkRegExp, checkTypes, checkVars, const, ' +
-        'constantProperty, deprecated, duplicate, es5Strict, ' +
-        'fileoverviewTags, missingProperties, nonStandardJsDocs, ' +
-        'strictModuleDepCheck, suspiciousCode, undefinedNames, ' +
-        'undefinedVars, unknownDefines, unusedLocalVariables, uselessCode, visibility}\n' +
-        ' */\n' + code + '\n' +
-        'ol.ext.' + mod.name + ' = module.exports;\n' +
-        '})();\n';
   };
-
-  if (mod.browserify) {
-    browserify(mod.main, {standalone: mod.name}).bundle(function(err, buf) {
-      if (err) {
-        callback(err);
-        return;
-      }
-      callback(null, wrap(derequire(buf.toString())));
-    });
-  } else {
-    fs.readFile(mod.main, function(err, data) {
-      if (err) {
-        callback(err);
-        return;
-      }
-      callback(null, wrap(data.toString()));
-    });
-  }
 }
 
-
-/**
- * Build external modules.
- * @param {Array.<Object>} modules External modules.
- * @param {function(Error)} callback Called with any error.
- */
-function buildModules(modules, callback) {
-  async.each(modules, function(mod, done) {
-    var output = path.join(buildDir, mod.name) + '.js';
-    async.waterfall([
-      wrapModule.bind(null, mod),
-      fs.outputFile.bind(fs, output)
-    ], done);
-  }, callback);
-}
-
-
 /**
  * Build all external modules.
- * @param {function(Error)} callback Called with any error.
+ * @return {Promise} Resolves on successful completion.
  */
-function main(callback) {
-  var modules = getExternalModules();
-  buildModules(modules, callback);
+function main() {
+  return Promise.all(pkg.ext.map(ext => {
+    const moduleName = ext.name || ext.module;
+    const options = {
+      entry: require.resolve(ext.module),
+      dest: `${path.join(__dirname, '..', 'build', 'ol.ext', moduleName.toLowerCase())}.js`,
+      format: 'iife',
+      moduleName: moduleName,
+      exports: 'named',
+      plugins: [
+        node(),
+        common(),
+        cleanup(),
+        wrap(ext)
+      ]
+    };
+    return rollup(options).then(bundle => bundle.write(options));
+  }));
 }
 
 if (require.main === module) {
-  main(function(err) {
-    if (err) {
-      process.stderr.write(err.message + '\n');
-      process.exit(1);
-    } else {
-      process.exit(0);
-    }
+  main().catch(err => {
+    process.stderr.write(`${err.message}\n`, () => process.exit(1));
   });
 }
 

test/spec/ol/format/mvt.test.js

@@ -1,8 +1,8 @@
 goog.provide('ol.test.format.MVT');
 
 goog.require('ol.Feature');
-goog.require('ol.ext.pbf');
-goog.require('ol.ext.vectortile');
+goog.require('ol.ext.PBF');
+goog.require('ol.ext.vectortile.VectorTile');
 goog.require('ol.format.MVT');
 goog.require('ol.geom.Point');
 goog.require('ol.render.Feature');
@@ -40,7 +40,7 @@ where('ArrayBuffer.isView').describe('ol.format.MVT', function() {
         featureClass: ol.Feature,
         layers: ['poi_label']
       });
-      var pbf = new ol.ext.pbf(data);
+      var pbf = new ol.ext.PBF(data);
       var tile = new ol.ext.vectortile.VectorTile(pbf);
       var geometry, rawGeometry;
 

transforms/module.js

@@ -1,5 +1,5 @@
-const pkg = require('../package.json');
-const version = require('../package/package.json').version;
+const parentPackage = require('../package.json');
+const thisPackage = require('../package/package.json');
 
 const defines = {
   'ol.ENABLE_WEBGL': false
@@ -16,20 +16,30 @@ function resolve(fromName, toName) {
   if (toParts[0] === 'ol' && toParts[1] === 'ext') {
     let name = toParts[2];
     let packageName;
-    for (let i = 0, ii = pkg.ext.length; i < ii; ++i) {
-      const dependency = pkg.ext[i];
-      if (dependency.module === name) {
-        packageName = name;
-        break;
-      } else if (dependency.name === name) {
+    let imported;
+    for (let i = 0, ii = parentPackage.ext.length; i < ii; ++i) {
+      const dependency = parentPackage.ext[i];
+      imported = dependency.import;
+      if (dependency.module === name || dependency.name === name) {
         packageName = dependency.module;
+        // ensure dependency is listed on both package.json
+        if (
+          !thisPackage.dependencies[packageName] ||
+          thisPackage.dependencies[packageName] !== parentPackage.dependencies[packageName]
+        ) {
+          throw new Error(`Package ${packageName} must appear in all package.json at the same version`);
+        }
         break;
       }
     }
     if (!packageName) {
       throw new Error(`Can't find package name for ${toName}`);
     }
-    return packageName;
+    if (imported) {
+      return [packageName, imported];
+    } else {
+      return packageName;
+    }
   }
   const fromLength = fromParts.length;
   let commonDepth = 1;
@@ -122,7 +132,7 @@ module.exports = function(info, api) {
   // replace `ol.VERSION = ''` with correct version
   root.find(j.ExpressionStatement, getMemberExpressionAssignment('ol.VERSION'))
     .forEach(path => {
-      path.value.expression.right = j.literal(version);
+      path.value.expression.right = j.literal(thisPackage.version);
     });
 
   const replacements = {};
@@ -186,12 +196,18 @@ module.exports = function(info, api) {
       }
       const renamed = rename(name);
       replacements[name] = renamed;
-      imports.push(
-        j.importDeclaration(
-          [j.importDefaultSpecifier(j.identifier(renamed))],
-          j.literal(resolve(provide, name))
-        )
-      );
+      const resolved = resolve(provide, name);
+      let specifier, source;
+      if (Array.isArray(resolved)) {
+        // import {imported as renamed} from 'source';
+        specifier = j.importSpecifier(j.identifier(resolved[1]), j.identifier(renamed));
+        source = resolved[0];
+      } else {
+        // import renamed from 'source';
+        specifier = j.importDefaultSpecifier(j.identifier(renamed));
+        source = resolved;
+      }
+      imports.push(j.importDeclaration([specifier], j.literal(source)));
     })
     .remove();