orbitjs / orbit

Composable data framework for ambitious web applications.
https://orbitjs.com
MIT License
2.33k stars 134 forks source link

@orbit/record-cache with ESM not working #861

Closed Thomasan1999 closed 3 years ago

Thomasan1999 commented 3 years ago

When I try to import Orbit packages as ES modules, I get the following error: record-transform-buffer.ts:33 Uncaught TypeError: Class extends value undefined is not a constructor or null. It is related to the following line: https://github.com/orbitjs/orbit/blob/fdb35dcc6f774a276387ab8593ef97d6b1c6dd44/packages/%40orbit/record-cache/src/record-transform-buffer.ts#L9 If I change the import path to '.', it works.

dgeb commented 3 years ago

Can you provide a reproduction in a simple public project please?

Thomasan1999 commented 3 years ago

I've found out that the error is triggered only when running Vite dev server, it might be caused by the order of imported files. It works in Vite build though.

I don't know if it's relevant for you anymore, if it is, here is the test project: https://github.com/Thomasan1999/orbit-record-cache-import-test.

samwightt commented 3 years ago

I'm getting this error on Vite too. I'm using Yarn 2 though, so I'm not sure if that's related to the error or not.

dgeb commented 3 years ago

@Thomasan1999 thanks for the test repo, I will investigate soon. I'm glad it seems to be working in Vite build.

@tchak as a Vite user, do you have any ideas here?

Thomasan1999 commented 3 years ago

@dgeb Meanwhile, I've opened another issue in the Vite repo. According to sodatea, it might be related to circular dependencies. A similar issue here.

@samwightt Apparently, if any package provides ESM build, you can exclude them from dependency pre-bundling. @orbit packages provide ESM, so you can use this as a workaround.

import { defineConfig } from "vite";

export default defineConfig({
  optimizeDeps: {
    exclude: [
      '@orbit/indexeddb',
      '@orbit/memory',
      '@orbit/records'
    ]
  }
})
dgeb commented 3 years ago

@Thomasan1999 thanks for tracking this down. I can confirm that there is a circular dependency between sync-record-cache.ts and record-transform-buffer.ts in @orbit/record-cache. This cycle would not be trivial to remove, but it is plausible, and I'll consider it. I'm glad that there is an alternate workaround for now.

dgeb commented 3 years ago

I decided to remove the cycle in #868 because it cleaned up several aspects of the implementation (see the PR for details). This has been resolved in:

You can confirm that this fixes the vite/esbuild issue by changing the deps in your test repo to:

-        "@orbit/indexeddb": "0.17.0-beta.21",
-        "@orbit/memory": "0.17.0-beta.21",
-        "@orbit/records": "0.17.0-beta.21"
+        "@orbit/indexeddb": "0.17.0-beta.23",
+        "@orbit/memory": "0.17.0-beta.23",
+        "@orbit/records": "0.17.0-beta.22"

Thanks again for your help tracking this down!

Thomasan1999 commented 3 years ago

I updated the deps in the test repo and in the project where I actually use them. It works in both now.