search

flightcontrolhq / superjson

Safely serialize JavaScript expressions to a superset of JSON, which includes Dates, BigInts, and more.
https://www.flightcontrol.dev?ref=superjson
MIT License
4.02k stars 87 forks source link

Ambiguous class : provide a unique identifier #208

Closed LaCocoRoco closed 1 year ago

LaCocoRoco commented 1 year ago

Issues Reference: #105 #106 #107 #116 #118 Code Reference: superjson/blob/main/src/registry

This warning only happens in none production environment. In my use case by using next-superjson-plugin package with a custom class register in nextjs app.ts. Can someone explain why this warning exits or better how i can fix it in development? If i'm not wrong this happens because the registerClass gets executed more then once? I could not find any flag or possibility to suppress this warning.

Skn0tt commented 1 year ago

Hi @LaCocoRoco! Yes, this warning happens because registerClass is executed more than once for a class of the same name. It's meant to uncover potential bugs in your module loading, and there's no way of supressing them. Could you elaborate on when exactly you're seeing the warning? Is it during Hot Module Replacement? Reproduction steps / a repro repo would be greatly appreciated.

LaCocoRoco commented 1 year ago

Hi @Skn0tt, here is a sandbox to reproduce the problem.

  1. Start project in dev mode
  2. Open the preview
  3. Change the value in pages/index.tsx
  4. Error Ambiguous class should be printed in the console

Probably fast refresh from the nextjs framework is the origin of this issue, because on every change the page gets reloaded. I think it is neglectable, but perhaps there is a way to fix it.

Skn0tt commented 1 year ago

I'm seeing it, thank you! Yeah, that looks like it's occuring during Hot Module Replacement (HMR). That means there already was a class called Foo, and now the Next.js Dev Server dynamically loads in the updated module, which has a new version of the class Foo, which triggers the warning.

The warning was originally supposed to warn about existing classes being overwritten, e.g. when something in the build process fails and e.g. some client stub is placed in the server bundle. It's not supposed to trigger during HMR, but apparently it does, and I don't see a good way of excluding HMR here 🤔 Maybe we should disable the warning altogether. What do you think?

LaCocoRoco commented 1 year ago

Depends if there are any significant negative effects which could happen if you run in a real ambiguous class situation. But if it is a simple information for "you unnecessary load the class more then once" in my personally opinion you probably could disable it.

Skn0tt commented 1 year ago

Removed the warning in https://github.com/blitz-js/superjson/releases/tag/v1.12.0.