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

work around webpack build warning with earcut #4097

Merged
merged 1 commit into from
Aug 5, 2016

Conversation

chris-cooper
Copy link
Contributor

Pulling in Cesium via webpack produces the following warning...

WARNING in ./~/cesium/Source/ThirdParty/earcut-2.1.1.js
Critical dependencies:
1:479-486 This seems to be a pre-built javascript file. Though this is possible, it's not recommended. Try to require the original source to get better results.
 @ ./~/cesium/Source/ThirdParty/earcut-2.1.1.js 1:479-486

This PR strips the boilerplate from earcut so it can be loaded directly like other Cesium modules.

@pjcozzi
Copy link
Contributor

pjcozzi commented Jul 7, 2016

Thanks for the fix, @chris-cooper!

@bagnell instead, is it possible to just use the unminified version of earcut?

@mramato
Copy link
Contributor

mramato commented Jul 14, 2016

While we should always use the unminified version of libraries, if I recall correctly, there was no unminified version of earcut that did not include a bunch of debug code. @bagnell can you please confirm.

@mramato
Copy link
Contributor

mramato commented Jul 14, 2016

Rather than modify the minified version, I'd rather see us modify an unminified version to not include it's debug code (or use pragmas) if possible.

@chris-cooper
Copy link
Contributor Author

Thanks @mramato. A couple of things I forgot to mention:

@kring
Copy link
Member

kring commented Jul 19, 2016

@chris-cooper I'm seeing the warningin 1.23, too, but I'm not seeing any runtime errors. When do you see that? My test case it is admittedly pretty simple: I'm loading a GeoJSON with a polygon. I would expect that to trigger it, though.

@chris-cooper
Copy link
Contributor Author

It was working OK locally, but intermittently gives the following error when running from a remote server...

earcut.js:645 An error occurred while rendering.  Rendering has stopped.
undefined
Error: importScripts failed for Workers/createPolylineGeometry at https://(url-changed)/cesium/Workers/createPolylineGeometry.js
http://requirejs.org/docs/errors.html#importscripts
Error: importScripts failed for Workers/createPolylineGeometry at https://(url-changed)/cesium/Workers/createPolylineGeometry.js
http://requirejs.org/docs/errors.html#importscripts
    at makeError (https://(url-changed)/cesium/Workers/cesiumWorkerBootstrapper.js:29:1191)
    at Function.req.load (https://(url-changed)/cesium/Workers/cesiumWorkerBootstrapper.js:29:16165)
    at Object.load (https://(url-changed)/cesium/Workers/cesiumWorkerBootstrapper.js:29:12786)
    at Object.load (https://(url-changed)/cesium/Workers/cesiumWorkerBootstrapper.js:29:6357)
    at Object.fetch (https://(url-changed)/cesium/Workers/cesiumWorkerBootstrapper.js:29:6296)
    at Object.check (https://(url-changed)/cesium/Workers/cesiumWorkerBootstrapper.js:29:7362)
    at Object.enable (https://(url-changed)/cesium/Workers/cesiumWorkerBootstrapper.js:29:9429)
    at Object.enable (https://(url-changed)/cesium/Workers/cesiumWorkerBootstrapper.js:29:11891)
    at Object.<anonymous> (https://(url-changed)/cesium/Workers/cesiumWorkerBootstrapper.js:29:9286)
    at https://(url-changed)/cesium/Workers/cesiumWorkerBootstrapper.js:29:938

@mramato
Copy link
Contributor

mramato commented Aug 3, 2016

I looked into this some more and I think the easiest thing to do is just remove the source map directive from the submitted file. The "build" we are using is nothing more than the commonjs source file that @chris-cooper mentioned but with a UMD wrapper, so removing the source map should fix the issue without introducing any problems (or making it hard to maintain). I'll open a PR for people to try out.

@mramato
Copy link
Contributor

mramato commented Aug 3, 2016

@chris-cooper can you give #4169 a try and confirm it fixes the issue? Thanks.

@mramato
Copy link
Contributor

mramato commented Aug 5, 2016

There was additional discussion of this in #4169, ultimately it is a webpack issue but there's no reason we can't work around it since we know Cesium sues AMD and there's significant maintenance overhead here.

@mramato
Copy link
Contributor

mramato commented Aug 5, 2016

Thanks @chris-cooper

@mramato mramato merged commit 2b8620f into CesiumGS:master Aug 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants