MasterKale / SimpleWebAuthn

WebAuthn, Simplified. A collection of TypeScript-first libraries for simpler WebAuthn integration. Supports modern browsers, Node, Deno, and more.
https://simplewebauthn.dev
MIT License
1.5k stars 127 forks source link

Switch to ESM #136

Closed florianbepunkt closed 3 years ago

florianbepunkt commented 3 years ago

I see that this lib switched to ESM only. This has some impact, considering the current state of the JS ecosystem. For example Next.js still has no support for ESM modules. While ESM is the way forward for sure, I wonder if it would be possible to still offer a CJS build until the the JS ecosystem more widely has adapted ESM.

MasterKale commented 3 years ago

Hmm, I successfully got a Next.js project to incorporate the ESM bundle just fine here. Can you help me understand your setup that's having trouble?

I'll admit I'm no Next.js expert but I was satisfied from testing that the move to ESM-only wouldn't impact most of the popular frameworks. Let me take a look at how I got Next.js to handle it, I should still have that around here somewhere...

MasterKale commented 3 years ago

Okay, I found my "Next.js + JS" setup, here's the package.json:

{
  "name": "learn-starter",
  "version": "0.1.0",
  "private": true,
  "scripts": {
    "dev": "next dev",
    "build": "next build",
    "start": "next start"
  },
  "dependencies": {
    "@simplewebauthn/browser": "file:../browser",
    "next": "^10.0.0",
    "react": "17.0.1",
    "react-dom": "17.0.1"
  }
}
$> npm list --depth=0
learn-starter@0.1.0 /Users/matt/Repos/simplewebauthn-integration-testing/nextjs-js-skoshx
├── @simplewebauthn/browser@3.0.0 -> /Users/matt/Repos/simplewebauthn-integration-testing/browser
├── next@10.2.3
├── react-dom@17.0.1
└── react@17.0.1

It's nothing special, just a barebones Next.js v10 project with a copy of the ESM version of @simplewebauthn/browser. npm start says it's using Webpack 5 to build; maybe this will become a question of ESM support with earlier versions of Next.js using Webpack 4?

florianbepunkt commented 3 years ago

@MasterKale Strange. I have @simplewebauthn/browser as a dependency of an auth library that is consumed by a Next.js app. Using the library with @simplewebauthn/browser works fine, but when it is included and I start next dev I get the following error

SyntaxError: Unexpected token 'export'
    at wrapSafe (internal/modules/cjs/loader.js:979:16)
    at Module._compile (internal/modules/cjs/loader.js:1027:27)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.Module._load (internal/modules/cjs/loader.js:769:14)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at Object.<anonymous> (/Users/florian/Github Repositories/supportcodes/node_modules/@culinario-mortale/authentication-internal/lib/auth-stages/sign-in/sign-in.logic.js:19:19)
    at Module._compile (internal/modules/cjs/loader.js:1063:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
/Users/florian/Github Repositories/supportcodes/node_modules/@simplewebauthn/browser/dist/bundle/index.js:116
export { startAssertion, startAttestation, supportsWebauthn };

I'm using Next 11 + Webpack 6, no special config.

Using @simplewebauthn/browser@3.0.0 the error is gone.

Not exactly sure what is the root of the problem. As far as I known, Next.js does not support ESM modules (https://github.com/vercel/next.js/discussions/22381). Problem could also be my build setup

In the library that consumes @simplewebauthn/browser I have the following build setup

// package.json  (relevant parts) – no module prop set, only main
{
"main": "./lib/index.js",
 "name": "@culinario-mortale/authentication-internal",
"types": "./lib/index.d.ts",
"files": [
    "lib"
  ],
"scripts": {
"build": "rimraf lib && tsc",
}
}
// tsconfig.json
{
  "compilerOptions": {
    "allowSyntheticDefaultImports": true,
    "declaration": true,
    "esModuleInterop": true,
    "jsx": "react",
    "lib": ["ES2018", "DOM"],
    "module": "CommonJS",
    "moduleResolution": "node",
    "outDir": "lib",
    "strict": true,
    "suppressImplicitAnyIndexErrors": true,
    "target": "ES6"
  },
  "include": ["src"],
  "exclude": [
    "node_modules",
    "lib",
    "./src/**/*.spec.ts",
    "./src/**/*.spec.tsx",
    "./src/setupTests.ts",
    "./src/**/__mocks__"
  ]
}
MasterKale commented 3 years ago

Are you able to produce a reproduction repo I can pull down to work with? I have a couple of hunches but if you're able to distill the issue down to its essential elements I'll have a much easier time identifying potential solutions.

florianbepunkt commented 3 years ago

I will try to put something together this week, give me a few days.

abartolo commented 3 years ago

I am also getting this error on my Next project. Currently reverting to 3.0.0 version works fine :)

MasterKale commented 3 years ago

I have @simplewebauthn/browser as a dependency of an auth library that is consumed by a Next.js app. Using the library with @simplewebauthn/browser works fine, but when it is included and I start next dev I get the following error

@florianbepunkt I keep thinking about this. So @simplewebauthn/browser can be consumed directly by Next.js 10+, but the same project could fail to build if browser is a dependency of another library that's being consumed by Next.js instead?

florianbepunkt commented 3 years ago

@MasterKale Apologies for the delay. I contrived a repro.

This is the library that imports webauthn (published to npm): https://github.com/florianbepunkt/test-webauthn-package-next This is the next app that consumes the above lib: https://github.com/florianbepunkt/webauthn-next

You can clone the next app and run npm install && npm run dev

akanass commented 3 years ago

@MasterKale The problem is easy to understand.

His library is compiled in CJS and it's doing only an import of SWAN and using startAssertion method.

Then that library is included in a Next.js project but because the library isn't in ESM so it's not compiled correctly in the application.

The library which included SWAN should be compiled and published in ESM then included in the Next.js application like this all will be compiled and all will work.

The problem isn't related to SWAN itself but to the usage of it and the understanding how to compile and publish a library to be compatible with bundler in an application.

florianbepunkt commented 3 years ago

@akanass Yes, but as far as I know, Next.js does not support ESM only modules

For example Next.js still has no support for ESM modules. While ESM is the way forward for sure, I wonder if it would be possible to still offer a CJS build until the the JS ecosystem more widely has adapted ESM.

akanass commented 3 years ago

@florianbepunkt Next.js is able to compile ESM to CJS as @MasterKale did in his example.

All is working fine with ESM in this library because it's the job of the application's bundler to compile to CJS.

We did a lot of test which worked fine on our side even with Next.js

You just have to try to put SWAN directly in Next without your lib and you will see that it's working

florianbepunkt commented 3 years ago

@akanass Thank you. I definitely see, that importing this package directly in Next.js works. However I can't figure out where I went wrong...

There is no compley build setup, just using plain typescript compiler tsc in my example.

I just updated the tsconfig to match the one of this package, so it does not compile to CJS, but then I get an error from next.js

// tsconfig of my lib
{
  "compilerOptions": {
    "allowSyntheticDefaultImports": true,
    "declaration": true,
    "esModuleInterop": true,
    "jsx": "react",
    "target": "ES2018",
    "module": "ES2020",
    "moduleResolution": "node",
    "lib": ["ES2018", "DOM"],
    "outDir": "lib",
    "strict": true,
    "suppressImplicitAnyIndexErrors": true
  },
  "include": ["src"],
  "exclude": [
    "node_modules",
    "lib",
    "./src/**/*.spec.ts",
    "./src/**/*.spec.tsx",
    "./src/setupTests.ts",
    "./src/**/__mocks__"
  ]
}

Next error: SyntaxError: Unexpected token 'export'

akanass commented 3 years ago

@florianbepunkt the problem isn't with Next options but with your library which has to be compiled in ESM as well because that is where SWAN is imported.

Then you include your ESM library in Next and it will be compiled in CJS and it should work with the default option that you used when you have directly imported SWAN inside.

florianbepunkt commented 3 years ago

But the tsconfig (of my library) given above does compile to an ESM module, doesn't it?

"compilerOptions": {
"target": "ES2018",
"module": "ES2020",
 "moduleResolution": "node",
}
MasterKale commented 3 years ago

@akanass Thanks for jumping in, now that we know it's likely something with the library and how it's built I'll work with @florianbepunkt from here on.

@florianbepunkt Let me summarize as I understand things so far: you confirmed that @simplewebauthn/browser works when directly imported, but your library (that uses browser), when built with tsc, succeeds in building to ESM but Next.js errors out about the existence of export when you attempt to import your library?

After hearing that you use tsc to transpile your library I need to clarify that browser uses Rollup to build to ESM (getting pedantic for a second, tsc lacks the ability to bundle code; it can only compile TS to vanilla JS code), so this won't be an apples-to-apples comparison.

When Rollup builds browser it generates a single file with everything crammed into it, and a single export statement at the bottom - you can see for yourself here: https://unpkg.com/@simplewebauthn/browser@3.1.0/dist/bundle/index.js

When I use just tsc to build I get a bunch of individual .js files that look like proper ESM code, minus the typings and I guess some TS optimizations (I haven't done a diff of a .ts compared to it's .js, just eyeballed it)

So for some reason Next.js has no problem with the export in something bundled with Rollup, but doesn't like it in TS code...

I think there are two options here:

  1. Figure out why Next.js supports an ESM bundle like browser
  2. Incorporate a bundler into your library and see if you can get it to produce something that Next.js will load

I suppose option three is I restore the CommonJS bundle to browser as was available pre-3.1.0, but I think it's weird for an ostensibly front end library to include a bundle using the same import/export mechanism as Node...

florianbepunkt commented 3 years ago

@MasterKale Thank you. Next.js by default only transpiles the ES6 code of the application source code (using babel-loader), while ES6 code imported from node_modules is not transpiled. This can be solved with the next-transpile-modules library.

// inside next.config.js

const withTM = require("next-transpile-modules")([
  "my-library",
  "@simplewebauthn/browser",
]);

module.exports = withTM({
  // your next.js config options
});
MasterKale commented 3 years ago

Thank you for the follow-up, and for including a solution! I'll look to document this officially somehow, because I know this question will come up again.

For anyone coming to this later: next-transpile-modules is the way to get Next.js to consume any third-party package that only offers an ESM build, like @simplewebauthn/browser.