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

Edge won't parse JS code that uses import.meta.url #8251

Closed
mramato opened this issue Oct 3, 2019 · 5 comments · Fixed by #8314
Closed

Edge won't parse JS code that uses import.meta.url #8251

mramato opened this issue Oct 3, 2019 · 5 comments · Fixed by #8314

Comments

@mramato
Copy link
Contributor

mramato commented Oct 3, 2019

We use import.meta.url in buildModuleUrl to find the current module location and build CESIUM_BASE_URL. Apparently Edge won't even parse code with import.meta.url in it and silently fails without any kind of indication.

We may be able to avoid import.meta.url completely and take a different approach.

There's some other weirdness in buildModuleUrl as well that we may want to review since a lot has changed since it was written.

@mramato
Copy link
Contributor Author

mramato commented Oct 3, 2019

This only affects the unbuilt versions of Cesium Viewer and Sandcastle on Edge. Built versions work as expected.

The main issue is that we ship the unbuilt versions in the Cesium release zip, the hosted versions on cesiumjs.org work fine.

@hpinkos
Copy link
Contributor

hpinkos commented Oct 15, 2019

@mramato is this something you're going to take care of?

@mramato
Copy link
Contributor Author

mramato commented Oct 16, 2019

I've looked into it, but at first glance Edge looks to be in worse shape than I thought. I was thinking of addressing this by having the Sandcastle we ship with Cesium always use CesiumUnminified instead of the built version, which would make it faster and work in Edge too (which is the only browser it currently has problems with).

@hpinkos
Copy link
Contributor

hpinkos commented Oct 22, 2019

@mramato Should this still be marked next-release? What's the plan?

mramato added a commit that referenced this issue Oct 24, 2019
import.meta.url is only at stage 3 of the standards and tools like webpack
and babel do not have good support. Not all browser do either.  I ran into
problems just trying to update the cesium-webpack-example because of it
and Edge won't even part JS code with import.meta.url in it.

This PR removes import.meta.url usage and just does a little extra work
so that all of our developer setups properly set CESIUM_BASE_URL. Most ES6
development setups would require setting CESIUM_BASE_URL anyway, so this
doesn't change much of anything for our end users.

Folks using combined versions of Cesium.js are unaffected.

I also tightened up our regex for `getBaseUrlFromCesiumScript` because
Cesium.js is always named Cesium.js if it is available, which wasn't the
case years ago when this code was written.  This should prevent accidental
detection of the wrong script, for example `CesiumViewer.js` would get
selected as a false positive.  This type of check is only valid for
non-ES6 module usage anyway, since there is no `script` tag inserter for
individual ES6 modules.

Fixes #8251 You can now load Cesium Viewer unbuilt in Edge (but it takes
forever)
@thw0rted
Copy link
Contributor

To those watching: did you ever wind up digging into ways to work around the SyntaxError? I was just reading over webpack/webpack#7353 which talks about https://webpack.js.org/guides/asset-modules/#url-assets.

The short version is that Webpack is able to statically analyze new URL("./somefile.ext", import.meta.url) and magically add the correct file to the bundle. They're recommending it as a path forward for libraries to reference their own assets in a fully cross platform way, which will work whether the module is included via a <script type="module"> tag directly, imported from a consumer that's run through a bundler (well, Webpack 5+ at least), or in Node as MJS, all using the same code.

This issue actually means there's two problems: this one, i.e. writing a polyfill/fallback that avoids the SyntaxError, and also the root cause of #8401, which is that the way Cesium handles asset references has too much indirection to support static analysis. I'm also commenting over there to explain what I mean by that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants