-
-
Notifications
You must be signed in to change notification settings - Fork 969
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(web): fix issues with certain bundlers only containing default
…
#2299
base: main
Are you sure you want to change the base?
Conversation
…on the imported shaka module doublesymmetry#2292
@Bram-dc , can you take a look at this PR and see if it solves your issue? |
@Bram-dc , did you get a chance to test this? |
I'm on vacation for three weeks, but I'll test it with Expo as soon as I get back:) |
No I have not, I can do tomorrow. |
No, it does not unfortunately solve the issue. Maybe I should open a PR at shaka player to also create an esm build. |
@Bram-dc it works in the demo app that you shared with me. Did you test it there (that's where I confirmed it)? |
Make sure to disable the caching of the vite bundler when testing. It probably saved an earlier "optimized" version, as vite calls it, of the package. "optimized" meaning it created a transformed version. Sometimes it does do it, sometimes not, but when it does not transform the shaka-player package errors occur. Adding the following to the Vite config, forces Vite to always transform the package with ESbuild.
Works fine with the current way we do it now, so I suggest we close this PR for now, and people that use this specific project structure can do it this way. |
@Bram-dc it works for me on the first run from scratch ( |
You can test it when the package is not cached using this:
|
It also does this when running the bundler without any include. It probably does it, but not always. |
https://vitejs.dev/guide/dep-pre-bundling
Maybe it is the dynamic import that confuses Vite. I am not really sure, and would have to check a later time. |
@Bram-dc yea, its still working for me. jspizziri/rntp-web-demo@5b0598d |
didn't seem to fix the issue for me neither, could we use require instead of the dynamic import here for now to avoid this issue? |
@andordavoti no, as I've already mentioned, that would break other things. Also, have you tested and confirmed that a static require fixes the expo issue? Despite what @Bram-dc reported this fix does actually solve the issue for vite. So really the only remaining one would appear to be expo. Edit: if you could send me an example expo repo where this isn't working that would be helpful. |
…on the imported shaka module
#2292