-
Notifications
You must be signed in to change notification settings - Fork 17
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
Allow passing exact file path to loam.initialize() #58
Comments
Hey @attilaolah -- thanks for opening this! I think I'd like to try to understand a bit more about your use-case. Are you trying to use Loam via |
That's exactly what I'm doing! Here is an example code pen. What I'm doing is the following:
But there is more tricks to do: But then, All this monkey patching works, but is rather ugly. Instead, I would like to do something like this:
Instead of loading Overall, that would be a much nicer user experience, although it could be an overkill — you could just say that it is a requirement that all files, loam and gdal-js, should be hosted on the same origin as the web page executing the script. |
Hey @attilaolah Thanks for this codepen, I am having an issue where This is what it resulted in for loading loam for use in a Create React App application:
However @ddohler I replied in this issue as well as I thought using unpkg could solve my problem. (here is my original issue: #62) Thanks EDIT I managed to fix my error and initialize loam by just importing loam at the top of loadLoam.js
All of this seems to work now! |
Hey @attilaolah -- Sorry for the delay here; I still had trouble understanding the use-case the first time I read your comment, but now that I'm looking at it again, it's pretty clear. I think the tricky part will be getting the absolute path into the worker--the only way that the main Loam library can communicate with the worker is via message passing, so in order to inject an absolute path for
So I think the solution you came up with, hacky as it feels, is probably pretty close to how it would look with native Loam support, though obviously you wouldn't have had to implement it yourself if Loam already supported this 🙂. The other wrinkle I'm seeing here is that Loam's Web Worker handling is hand-written because there were no libraries at the time I started the project to make that part easier. Now there are things like ComLink (#49 ), and webpack version 5 has also added improved Web Worker support. So I'm hesitant to add a lot of complexity to the web worker handling before taking on #49 because it seems like it could be wasted effort if ComLink has its own way of handling the problem. But this is useful to know going in to #49 because I don't want to switch if using ComLink would make this problem harder to solve in the future. Anyway, I think my next focus now that 1.0.0 is released is going to be on developer quality-of-life issues, so I'll prioritize this and #49. |
Thanks! Splitting the worker in two, one that would just bootstrap the other, sounds like a reasonable way forward. Also, focusing on #49 first sounds good to me. |
@attilaolah I've implemented this and I'll release a new version with the changes soon (hopefully within the next two weeks)! Thanks for your help and input! |
This is implemented as of version 1.1.0. Sorry, forgot to close this when that was released! |
Oh coolies! Thank you! |
Because the file might be named differently, or because the file might be on a remote origin, in which case Chrome won't allow creating a web worker using a script that is on the remote origin.
The latter can be worked around by
fetch()
ing the file and creating an object URL, like so:Not the trailing
#
, which is really a hack so that whenloam.initialize()
appends the filename (loam-worker.js
), it would end up being ignored. But that just so happens to work with object URLs.I'd suggest adding a second parameter for backwards compatibility, for the filename, i.e. something like:
I'll send a PR later on, just wanted to note this down first.
The text was updated successfully, but these errors were encountered: