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

Avoid errors when requiring the browser bundle in Node #9749

Merged
merged 2 commits into from
Jun 3, 2020

Conversation

mourner
Copy link
Member

@mourner mourner commented Jun 2, 2020

Potentially fixes #4593 by adding a shim for the window object when it's not defined and a few guards when accessing it at bundle evaluation.

Launch Checklist

  • briefly describe the changes in this PR
  • write tests for all new functionality
  • manually test the debug page
  • apply changelog label ('bug', 'feature', 'docs', etc) or use the label 'skip changelog'
  • add an entry inside this element for inclusion in the mapbox-gl-js changelog: <changelog>Fix reference error when requiring the browser bundle in Node</changelog>

@MarkLyck
Copy link

MarkLyck commented Jun 2, 2020

You are my hero!

Copy link
Contributor

@ryanhamley ryanhamley left a comment

Choose a reason for hiding this comment

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

I can confirm that this fixes the simple case of importing GL JS into a Node project as well as a Next.js test app I made (I don't appear to even need to do the dynamic import anymore).

@mourner mourner merged commit 536513d into master Jun 3, 2020
@mourner mourner deleted the fix-self-is-not-defined branch June 3, 2020 08:23
@MarkyMOD
Copy link

MarkyMOD commented Jun 5, 2020

Sorry for the ignorance, but how do I access these changes? Are they in mapbox-gl v1.10.1 or 1.11.0-beta.1(I still get the self is not defined error)? I tried npm i mapbox/mapbox-gl-js#master, but that gave me a new error, where mapbox-gl was not recognized.

@mourner
Copy link
Member Author

mourner commented Jun 5, 2020

@MarkyMOD it will be a part of either v1.11.0-beta.2 or v1.11.0 final next week.

@ryanhamley
Copy link
Contributor

@MarkyMOD you should be able to use the master branch to test this out now. After installing it as you have already done, you'll need to do the following:

cd node_modules/mapbox-gl-js
yarn
yarn run build-prod-min

This installs Rollup and builds the mapbox-gl bundle so when you run your project, it should work.

@MarkyMOD
Copy link

MarkyMOD commented Jun 6, 2020

Thanks much @ryanhamley. Downloaded the source code, as the npm method didn't provide the rollup folder or rollup config, then had to transfer some dist/ files from v1.10.1 as react-mapbox-gl was looking for the .css file (dist/ only had mapbox-gl.js in there), during my Next build, which led to more errors. Anyways, is a great starting point/something to go off of, really appreciate it. Would just wait for the next release, but the client is anxious to review the map, so it goes.

@ryanhamley
Copy link
Contributor

@MarkyMOD yarn run build-css in the mapbox-gl folder will add the css file to dist. But we should have either another beta or a final release out this week that contains this change.

@MarkyMOD
Copy link

MarkyMOD commented Jun 8, 2020

@ryanhamley Tried that too, but received an error of "unexpected token of '.' " without a position number, within the created css file (same error as when taking the css file from v1.10.1). Am keeping the client updated, and just going to wait for the new release, all good.

sgolbabaei added a commit that referenced this pull request Jun 9, 2020
sgolbabaei added a commit that referenced this pull request Jun 10, 2020
* Cherry pick (#9757) to release-erie

* Cherry pick (#9753) to release-erie

* Cherry pick (#9749) to release-erie

* Cherry pick (#9748) to release-erie
@MarkyMOD
Copy link

So, webpack now gets past "self is not defined", but fails at mapbox-gl.css, when it is required by react-mapbox-gl. Am receiving the same error as my previous post: "unexpected token of '.' " I converted the file from a single line of CSS to something more readable, and it is failing at ".mapboxgl-map" (line 1), which seems to say that mapbox is not being recognized as a class/in CSS (even though the error type shows as "SyntaxError")?

Is anyone else experiencing this, or have any idea of how to fix? I'm in Next.js, and using react-mapbox-gl as a middle-layer.

@mwarren2
Copy link

It all works in Meteor after release 1.11.0

@wratte
Copy link

wratte commented Jun 16, 2020

Hi,
After installing the latest version, there is still some places in this library that prevent Node to run.
Could it be possible to fix them as well ?
Thanks

ReferenceError: window is not defined
at Object. ((mypath)\suggestions\index.js:59:1)
at Module._compile (internal/modules/cjs/loader.js:799:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:810:10)
at Module.load (internal/modules/cjs/loader.js:666:32)
at tryModuleLoad (internal/modules/cjs/loader.js:606:12)
at Function.Module._load (internal/modules/cjs/loader.js:598:3)
at Module.require (internal/modules/cjs/loader.js:705:19)
at require (internal/modules/cjs/helpers.js:14:16)
at Object. ((mypath)\node_modules@mapbox\mapbox-gl-geocoder\lib\index.js:3:17)
at Module._compile (internal/modules/cjs/loader.js:799:30)

@ryanhamley
Copy link
Contributor

@wratte It looks like the error is coming from the Geocoder plugin, not GL JS. Can you confirm that and open an issue in that repo? https://github.com/mapbox/mapbox-gl-geocoder

Alternatively, if you're trying to use geocoding functionality in Node (as opposed to rendering a map with a geocoder control), you can use our Geocoding API.

@wratte
Copy link

wratte commented Jun 16, 2020

I confirm that this is from mapbox-gl-geocoder that is using "suggestions" library which eagerly use window.

I oppened an issue case:
mapbox/mapbox-gl-geocoder#366

cowboyox pushed a commit to cowboyox/mapbox that referenced this pull request Oct 8, 2024
* Cherry pick (mapbox/mapbox-gl-js#9757) to release-erie

* Cherry pick (mapbox/mapbox-gl-js#9753) to release-erie

* Cherry pick (mapbox/mapbox-gl-js#9749) to release-erie

* Cherry pick (mapbox/mapbox-gl-js#9748) to release-erie
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ReferenceError: self is not defined
6 participants