etro-js / etro

Typescript video-editing framework for the browser
https://etrojs.dev
GNU General Public License v3.0
859 stars 86 forks source link

Can't resolve 'etro' on Next 13. #243

Closed stylessh closed 7 months ago

stylessh commented 1 year ago

I'm using Next 13 with app directory and typescript, when I install the module and import it it shows a not found error:

image

Typescript autodetection works anyways, it's a strange issue.

image

clabe45 commented 10 months ago

Hi @stylessh, I tried to reproduce with NextJS 13 and typescript, but it built fine. I'm new to NextJS, what's App Directory? Did you install etro with npm i etro?

rhythm1995 commented 8 months ago

the same as me.

clabe45 commented 8 months ago

Can you include the steps to reproduce the issue

yryrrf commented 8 months ago

Can you include the steps to reproduce the issue

just do npm i etro then import it in any folders

stylessh commented 8 months ago
  1. Create a next app. npx create-next-app.
  2. Install the modules. npm i
  3. Install etro. npm i etro
  4. Import etro anywhere on your app.
clabe45 commented 8 months ago

Thanks, I reproduced it locally, and the error is being thrown because etro is a client-only framework, so it can only be imported in the browser. The usual solution is to defer the import to the browser, meaning the module gets imported in the browser, not in the server. I can't find any NextJS documentation or unofficial tutorials for how to lazy load a client-only library. For some reason, dynamic() can only be used to import modules whose default export is a React component.

Does anyone with more NextJS experience know of a way to lazy load a non-React library?

The only workaround I see is replacing the browser field in etro's package.json with main. Bundlers usually only consider a module "client-only" if its browser field is set. Replacing it with main would most likely allow it to get imported in the server. However, etro relies on DOM libraries, so this could result in unforeseen runtime issues.

regg00 commented 7 months ago

Thanks @clabe45. I got the same issue in SvelteKit. Replacing browser with main in the package.json file worked.

clabe45 commented 7 months ago

Good to know, thank you, replacing browser with main sounds like a good idea. I'll try to get to it in the next few days, but if anyone wants to open a PR to do it that would be very helpful.

clabe45 commented 7 months ago

Replacing browser with main in NextJS resulted in the following error, because now we're trying to access DOM globals (such as document) in Node:

$ npm run build                                                                                                                                                                         21:07:51

> etro-test@0.1.0 build
> next build

   ▲ Next.js 14.1.0

   Creating an optimized production build ...
 ✓ Compiled successfully
 ✓ Linting and checking validity of types    
   Collecting page data  .ReferenceError: document is not defined
    at 9427 (/private/tmp/etro-test/.next/server/chunks/63.js:14:5794)
    at t (/private/tmp/etro-test/.next/server/webpack-runtime.js:1:127)
    at 2917 (/private/tmp/etro-test/.next/server/chunks/63.js:14:54463)
    at Function.t (/private/tmp/etro-test/.next/server/webpack-runtime.js:1:127)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async collectGenerateParams (/private/tmp/etro-test/node_modules/next/dist/build/utils.js:919:21)
    at async /private/tmp/etro-test/node_modules/next/dist/build/utils.js:1138:17
    at async Span.traceAsyncFn (/private/tmp/etro-test/node_modules/next/dist/trace/trace.js:151:20)

> Build error occurred
Error: Failed to collect page data for /_not-found
    at /private/tmp/etro-test/node_modules/next/dist/build/utils.js:1258:15
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {
  type: 'Error'
}

Does anyone know how to lazy-load non-UI client libraries (i.e., packages that do not export a React component) in NextJS? This would solve the issue.

stylessh commented 7 months ago

@clabe45 usually importing it on useEffect hook.

useEffect(() => {
 const MyLibrary = import('etro').then(...)
}, [])

or importing your component with dynamic and disable completely ssr.

// EtroComponent.tsx
import etro from 'etro';

export const EtroComponent = () => {
 // all logic
  return <></>
}

and on your root page/component:

import dynamic from 'next/dynamic';

const EtroComponent = dynamic(() => import('EtroComponent.tsx'), {
  loading: () => <p>Loading...</p>,
  ssr: false // disable Server Side Rendering
})

export const Page = () => {
  return <EtroComponent />
}
clabe45 commented 7 months ago

Thanks @stylessh, etro can now be imported in the client-side code of a NextJS project with the useEffect hook. The dynamic example results in a "document" is not defined error (it seems like etro is still being imported in Node). I hope this was just a typo in the example you provided and not an issue with etro.

I'll deploy these changes to npm shortly, thanks everyone 💯

clabe45 commented 7 months ago

Fixed in v0.12.1!

stylessh commented 7 months ago

thanks for all the efforts @clabe45 💪