fix(web): fix issues with certain bundlers only containing `default` …

[jspizziri]

…on the imported shaka module

https://github.com/doublesymmetry/react-native-track-player/pull/2292

[jspizziri]

@Bram-dc , can you take a look at this PR and see if it solves your issue?

[jspizziri]

@Bram-dc , did you get a chance to test this?

[andordavoti]

I'm on vacation for three weeks, but I'll test it with Expo as soon as I get back:)

[Bram-dc]

No I have not, I can do tomorrow.

[Bram-dc]

No, it does not unfortunately solve the issue. Maybe I should open a PR at shaka player to also create an esm build.

[jspizziri]

@Bram-dc it works in the demo app that you shared with me. Did you test it there (that's where I confirmed it)?

[Bram-dc]

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.

    optimizeDeps: {
        include: ['shaka-player/dist/shaka-player.ui'],
    },

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.

[jspizziri]

@Bram-dc it works for me on the first run from scratch (rm -rf node_modules/.vite). Are you sure you're testing it correctly? Please document the steps you're taking to try and run this patch.

[Bram-dc]

You can test it when the package is not cached using this:

    optimizeDeps: {
        exclude: ['shaka-player/dist/shaka-player.ui'],
    },

[Bram-dc]

It probably saved an earlier "optimized" version, as vite calls it, of the package.

It also does this when running the bundler without any include. It probably does it, but not always.

[Bram-dc]

https://vitejs.dev/guide/dep-pre-bundling

During development, Vite's dev serves all code as native ESM. Therefore, Vite must convert dependencies that are shipped as CommonJS or UMD into ESM first. When converting CommonJS dependencies, Vite performs smart import analysis so that named imports to CommonJS modules will work as expected

Maybe it is the dynamic import that confuses Vite. I am not really sure, and would have to check a later time.

[jspizziri]

@Bram-dc yea, its still working for me. https://github.com/jspizziri/rntp-web-demo/commit/5b0598d03634dfc59f6d7120d0c30d7fe29570f6

[andordavoti]

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?

[jspizziri]

@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.

[andordavoti]

Sorry for the late response @jspizziri. I've not tested with a static require, it's just an assumption, should've tested that before suggesting it, sorry about that.

I created a repro with a simple expo app. If you launch it on web and look in the console you will get the error "Error: You must call setupPlayer prior to interacting with the player.".

GitHub repro: https://github.com/andordavoti/RNTP-WEB/tree/2299-fix-expo-web

[github-actions[bot]]

This PR is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 7 days.