-
-
Notifications
You must be signed in to change notification settings - Fork 797
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
Fix local resource error on browser (#392) #394
Conversation
Hmm, not so sure about this as we just started to use Maybe @nxtexe can provide some suggestions. 😃 |
Not sure what you mean here. |
I may just be doing something wrong since js and webdev were never my thing so I'm trying to learn as I go, but what I observed is that building the source into a .min.js will replace all "import.meta.url" with its value at compile time, so when running from browser it will have the import url hardcoded as "file:https://path/to/build/environment", which would be useless on all other systems since only the system that built the .min.js has that path, and also browsers don't allow direct file:https:// access at all. I can see how import.meta.url would be a good fit for the node side of things, it allows you to specify a corePath relative to where you are importing from, but in browser this makes it impossible to import on a relative path on the same domain: While the source shows: coreRemotePath = new URL(_corePath, import.meta.url) The build process will hardcode "import.meta.url" at compile time, so the browser effectively runs: coreRemotePath = new URL(_corePath, "file:https:///path/to/build/environment") So say my html is on localhost:3000/example.html, I have the core at localhost:3000/static/js/ffmpeg-core.js, the browser would end up running: coreRemotePath = new URL("static/js/ffmpeg-core.js", "file:https:///path/to/build/environment")
console.log(coreRemotePath)
// file:https:///path/to/build/environment/static/js/ffmpeg-core.js So even though I am running from a website on a web server, it tries to get the file locally instead of relatively to the path on the web server. Maybe there's a way to add support for import.meta.url when building, but to be honest importing seems more related to the node side of things as opposed to minified browser scripts... that's why my changes only affected the src/browser directory, with node it makes sense to use the import url but for browser it breaks the custom corePath feature at the moment. |
Well I see what you mean and you have a point. Referencing this issue it seems it's a webpack issue. It seems webpack is statically replacing references of |
Seems like we aren't the only ones with this issue 😄 It is indeed a sort of compile flag Will fix my PR in a bit |
Ok so it works and it doesnt, and im not so happy with this fix to be honest. Now it doesnt hardcode the local path, but the issue is that when importing the script into html it must be marked as type="module", and subsequently all other scripts that want to use ffmpeg must be marked as type="module"... |
Got it, maybe we can add some description in the README.md to address the usage? That should be helpful. 😄 |
Agreed. I personally got around my issue by passing the full path including domain by creating the url dynamically on my side: const ffmpeg = createFFmpeg({
...,
corePath: new URL("static/js/ffmpeg-core.js", document.location).href,
}); That could be an example perhaps... this way the user can handle it themselves and we don't have issues library wide |
Perhaps like that? |
LGTM! Thanks for the PR! |
Fix local resource error on browser (#392)
Using
import.meta.url
leads to bugs on browser since building will hardcode those as localfile:https://
urls. This fix usesdocument.location
instead.In particular this fixes #392 which made impossible specifying a custom
corePath
on the same domain.EDIT: this is now a more appropriate fix, import.meta.url is parsed instead of being hardcoded and webpack has been bumped in order to support this