muxinc / media-chrome

Custom elements (web components) for making audio and video player controls that look great in your website or app.
https://media-chrome.org
MIT License
1.21k stars 62 forks source link

Module parse error when importing react/media-store in Nextjs (v11) #888

Closed rssfrncs closed 4 months ago

rssfrncs commented 4 months ago

Attempting to load the react/media-store provider/hooks into a Next 11 app and getting a module parse error looks like it's trying to pull in typescript files?

../../node_modules/media-chrome/dist/react/media-store.tsx
Module parse failed: Unexpected token (2:12)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
| import { createContext, useContext, useEffect, useMemo } from 'react';
> import type { Context, ReactNode } from 'react';
| import React from 'react';
| import { useSyncExternalStoreWithSelector } from './useSyncExternalStoreWithSelector';
cjpillsbury commented 4 months ago

Hey @rssfrncs thanks for checking out media store! I'll take a look and see if we can get a fix out asap.

rssfrncs commented 4 months ago

Hey no problem i love the (redux-esque) design of it!

Aside - i wondered why you opted to export the MediaEvent types but not provide a typesafe way for the details property (and save having to refer back to the docsite)?

Either purely in typescript with a union like,

type MediaEvent = { type: 'mediaseekrequest'; details: number; } |  { type: 'mediatogglesubtitlesrequest'; details: boolean }

Or support js with event creators as in,

{ 
  MEDIA_SEEK_REQUEST: (details: number) => ({ type: 'mediaseekrequest', details }), 
  // ...other events 
}

// usage
dispatch(MediaActionTypes.MEDIA_SEEK_REQUEST(100);
cjpillsbury commented 4 months ago

looks like an artifact somehow made it into our dist from an interim state. https://cdn.jsdelivr.net/npm/media-chrome@3.2.1/dist/react/ running the build locally doesn't produce that. I'll have to do some digging into our ci/cd crud.

Your build env may yell at you, but just in case it doesn't, in the interim, can you try importing the non-tsx module directly, e.g.:

import { MediaProvider } from 'media-chrome/dist/react/media-store.js';

?

cjpillsbury commented 4 months ago

Aside - i wondered why you opted to export the MediaEvent types but not provide a typesafe way for the details property (and save having to refer back to the docsite)?

Either purely in typescript with a union like,

type MediaEvent = { type: 'mediaseekrequest'; details: number; } |  { type: 'mediatogglesubtitlesrequest'; details: boolean }

Or support js with event creators as in,

{ 
  MEDIA_SEEK_REQUEST: (details: number) => ({ type: 'mediaseekrequest', details }), 
  // ...other events 
}

// usage
dispatch(MediaActionTypes.MEDIA_SEEK_REQUEST(100);

With respect to the former option (full typescript event/action types), this is because of the pre-existing codebase that Media Store was extricated from. We're likely migrating the entire Media Chrome codebase over to TypeScript soon (currently we rely on JSDoc-declared types), so this will be much easier at that point.

With respect to the latter, it's really just because that "newer" approach to state updates can always be built on top of the "more primitive" implementation I went with. If enough folks want the "pre-canned methods" as part of the API for convenience, we can always add it.

rssfrncs commented 4 months ago
media-chrome/dist/react/media-store.js'

I get a bit further but looks like the js file is attempting to import the ts version of the sync external store hook?

../../node_modules/media-chrome/dist/react/useSyncExternalStoreWithSelector.ts
Module parse failed: Unexpected token (25:21)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
|  * https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/is
|  */
> function isPolyfill(x: any, y: any) {
|   return (
|     (x === y && (x !== 0 || 1 / x === 1 / y)) || (x !== x && y !== y) // eslint-disable-line no-self-compare
rssfrncs commented 4 months ago

it's working fine in a stackblitz vite project so i wonder if this is next (next 11) specific 😬

cjpillsbury commented 4 months ago

whatever the root cause, we should probably try to fix it regardless. Have you tried with any of the more new-fangled nextjs versions?

rssfrncs commented 4 months ago

Oh useSyncExternalStore is React 18+ right? This project is still on 17 so that will be an issue regardless.

Okay i can get useSyncExternalStore working by manually replacing with the use-external-store-sync official shim.

But it's still odd it is importing typescript files from dist i suppose?

cjpillsbury commented 4 months ago

re: useSyncExternalStore, yeah unfortunately we have a hard React v18+ dependency (though iirc you can upgrade to react 18 + next 11 and it should be fine?)

re: types crud - yeah if you take a look at the jsdelivr link I posted above, which is just the npm package version directory, you'll see that somehow our package dist ended up with some src artifacts in it. Since the imports don't specify an extension, different build tools will "default" to different modules. That's the crux of your pain, but it's based on something that should have never ended up in the package's dist in the first place.

cjpillsbury commented 4 months ago

ok kicked out a release (v3.2.2) that should fix it (🤞)! Whenever you get a chance, mind trying it out?

rssfrncs commented 4 months ago

That fixed the issues thanks. But using the standard custom elements for now as we are stuck on 17 atm.

cjpillsbury commented 4 months ago

@rssfrncs good to know. Given the dependency on useSyncExternalStore, it may be tricky to get backwards compatibility to React v17. When I get some spare cycles, I'll peep under the hood on the react side and on react-redux, just in case there are some fallback strategies that wouldn't require too much code or too much increase in complexity.

rssfrncs commented 4 months ago

Yeah libraries have been using the official polyfill use-sync-external-store

2Pacalypse- commented 3 months ago

Hey, sorry for reviving this old(-ish) issue, but I get a similar error when importing media-store (I'm using next v14.1.3 and React v18.2.0):

image

The weird thing is that this file that it seemingly can't find, is located right there on the disk 🤔

image

Would you have an idea of what's going on here? Thx!