jspm / project

Roadmap and management repo for the jspm project
161 stars 8 forks source link

@clerk/clerk-js support #268

Closed fagiani closed 1 year ago

fagiani commented 1 year ago

@guybedford Based on #134 I am reporting this one here in order to understand how to fix it.

Importing @clerk/clerk-js currently gives the error:

TypeError: Clerk is not a constructor at sandbox:39:19

Example based on module's docs:

https://jspm.org/sandbox#H4sIAAAAAAAAA31U32/TMBB+319xhId10hKvG1SoS6pudJsYVPwIBW2IB9e5tt4S27KdlYD2v+M4Sdk64CGxfHf+7vvuzo6fZZLZSiGsbJGPduJuQZqNdgDiAi0FtqLaoE2C0i7CV4F3WG5zHM2EX7OYNPvNEUELTII7jmsltQ2ASWFROIg1z+wqyfCOMwz9Zh+44JbTPDSM5pj0XYKYNAziucwqj/osDN0CcJl+mMIFCtTUSg1vihofplR571nGLcw+vRs6PVaZISHLLjS6MaqIuCTPZ4dfllcXp7eT6YGu0p+X6ay/Ymc379P06G1/iZ/LXJxP+PXgR3E9OF1OGJ1cmcuPJy5BGNZcIDZMc2WhLlwScE+hoMoX5pfn0RpNMGwNzjRmOepb4v/hTe0KNiTphp1QxXArcvwiOhpEByTjxjZGFx142Pv9Jp1hUuGjbH+BfuB2AUpLhua/NOoNYVLj+DA6iPpEyAxzPjdkruXaoCYtxoaOI7TT/esvJk2l6sK0TYSzFKYyK3OEdMULM+xa6ApoQMm8WvA8h4XrbeHDDLTZDKy5XcnSAn9wwpTKb3opXVDNoT+IjvbaRnV9oqYSDIxmyb+kogmbbKGpSY370Uun1xd8y1VrBaalMVLzJRdJQIUUVSFLE4y29T6akwbFDwl0El7X3YSFlsXT+ThuQAAIga+aW3SCuWir4i5U5i4tatwHVypqgIJBRd2oI7gKoi+WMwZOdgDUWs3npfNJAXaFHUpDMPJp3BU1Mscol8ue57V3vLFb8LTOtb/G2YnikMBuM4vfKlnqMJM1u+8Rk8Xuk3MuWOC6UdvbRmrT0DXlbbijQLNeN6tOfooWahtIZbmD9cKjqOF97wH+lD4mzaPh3hD/mP0G91AE7OQEAAA=

I appreciate any guidance towards fixing that.

Keep Rocking!

Bubblyworld commented 1 year ago

Hey, sorry for the delay - so it seems like this is actually an interop issue with CommonJS and ESM. @clerk/clerk-js is a CJS module, so if you import it from an ESM context (which is what you're doing in the sandbox) then the thing node.js does is to treat the CJS module as if it's wrapped in an ESM module that does this: export default module.exports;.

So import Clerk from "@clerk/clerk-js" means that Clerk is set to the whole module.exports object:

{ ClerkAPIResponseError: [Getter],
  MagicLinkError: [Getter],
  MagicLinkErrorCode: [Getter],
  default: [Getter],
  isClerkAPIResponseError: [Getter],
  isKnownError: [Getter],
  isMagicLinkError: [Getter],
  isMetamaskError: [Getter],
  parseError: [Getter],
  parseErrors: [Getter] }

That default key is actually the class that you're after. JSPM follows the same rules as node.js when transpiling to ESM, so that's why it's happening here as well. You can fix it by just using the default key:

  import Clerk from "@clerk/clerk-js";
    const clerkFrontendApi = 'clerk.[your-domain].com';
    const clerk = new Clerk.default(clerkFrontendApi);
    await clerk.load({
      // Set load options here...
    });

The reason the @clerk/clerk-js docs don't do this is because some bundlers don't follow the node.js interop rules and will set the CJS module's default export to module.exports.default if it exists. This inconsistency is quite a pain point, you can see some old discussions about it for rollup, for instance: https://github.com/rollup/plugins/issues/635

Bubblyworld commented 1 year ago

So I guess that's a no-fix from our side, since we stick quite closely to node.js semantics - ideally @clerk/clerk-js should export an ESM build alongside their CJS stuff with the correct exports (and then all bundlers/runtimes would behave the same way here).

fagiani commented 1 year ago

@Bubblyworld thanks a bunch for taking the time to investigate that. Being this a broader issue that could potentially affect other modules, would you agree with a README update that could have a short copy that can save some people's time? If so, I'd be willing to contribute.

fagiani commented 1 year ago

Something else I'd like to understand. After initializing the module, one of the possible usages of it is calling a function like:

Clerk.openSignIn();

no matter if i add .default in the middle, it will return:

TypeError: Clerk.default.openSignIn is not a function at sandbox:43:19

I've also tried to use the instantiated clerk on the main example without any luck.

Would you have any clues on how that would be approached?

Thanks again!

guybedford commented 1 year ago

@fagiani that would be more of an API issue to follow-up on, if you try importing the library in Node.js directly it would give the exact same semantics.

Closing as I believe this is resolved, but just let us know if we can help further.