mapbox / mapbox-gl-js

Interactive, thoroughly customizable maps in the browser, powered by vector tiles and WebGL
https://docs.mapbox.com/mapbox-gl-js/
Other
11.23k stars 2.23k forks source link

Flow reports errors about missing modules #7154

Open mikkom opened 6 years ago

mikkom commented 6 years ago

mapbox-gl-js version: 0.48

Steps to Trigger Behavior

  1. Update mapbox-gl and Flow to latest versions
  2. Add node_modules/mapbox-gl/flow-typed to [libs] section in .flowconfig
  3. Run Flow analysis

Link to Demonstration

The problem can be seen in repo https://github.com/mikkom/onnikka by running yarn && yarn flow.

Expected Behavior

No Flow errors (at least from within mapbox-gl).

Actual Behavior

Flow reports two errors where it cannot resolve a module.

Error ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ node_modules/mapbox-gl/src/util/window.js:5:16

Cannot resolve module gl.

     2│
     3│ import jsdom from 'jsdom';
     4│
     5│ import gl from 'gl';
     6│ import sinon from 'sinon';
     7│ import { extend } from './util';
     8│

Error ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ node_modules/mapbox-gl/src/util/window.js:6:19

Cannot resolve module sinon.

     3│ import jsdom from 'jsdom';
     4│
     5│ import gl from 'gl';
     6│ import sinon from 'sinon';
     7│ import { extend } from './util';
     8│
     9│ import type {Window} from '../types/window';

Manually adding gl and sinon as dependencies in the project removes both errors but I think that should not be necessary?

nfarina commented 6 years ago

I encountered this same issue and worked around it by adding a "stub" declaration for the modules I wasn't already importing myself. For instance, to fix the "gl not found", I created a file flow-typed/npm/gl.js with the contents:

// @flow

declare module 'gl' {
  declare export default Function
}

Now Flow is happy and thinks the module exists.

This does seem like something the end-user shouldn't have to concern themselves with though.

harto commented 6 years ago

I also see a reference to jsdom in src/util/window.js, which is only listed as a dev dependency in package.json.

boxman0617 commented 6 years ago
   8 |     id: number | string | void;
   9 | 
> 10 |     _vectorTileFeature: VectorTileFeature;
     |                         ^^^^^^^^^^^^^^^^^ Cannot resolve name `VectorTileFeature`.
  11 | 
  12 |     constructor(vectorTileFeature: VectorTileFeature, z: number, x: number, y: number) {
  13 |         this.type = 'Feature';

seeing the same thing on the latest v50.0

boxman0617 commented 6 years ago
[libs]
./node_modules/mapbox-gl/flow-typed
./node_modules/mapbox-gl/dist/mapbox-gl.js.flow

this fixed all of my flow issues with v50.0