ESM distro for @inrupt/solid-client-authn-browser

[riovir]

UPDATE 2: Here's a small reproduction on Codesandbox make sure to interact with the preview on a separate browser tab to avoid a login-loop.
UPDATE: Changed workaround to use a more idiomatic way to force ESM transitive dependencies for Rollup / Vite.

Search terms you've used

• esm
• rollup
• vite

Impacted environment

In which environment would the proposed feature apply?

• The browser (with a bundler)
• Node.js
• Other (please specify): ...
• I'm not sure.

Feature suggestion

The @inrupt/solid-client-authn-browser/package.json declares a valid ESM export. (With either an mjs extension or also the "type": "module" set).

Expected functionality/enhancement

It would be possible to use the package with modern bundlers, such as Vite.

Actual functionality/enhancement

When Vite does a production build the app seemingly works, but fails at runtime. The rabbit hole looks like this:

• Vite uses Rollup for production builds (instead of ESBuild used during development)
• Rollup concludes that @inrupt/solid-client-authn-browser is in CommonJS
• Then, when processing the transitive dependencies, eventually imports the @inrupt/solid-client-authn-core however, picks the CommonJS edition. This edition, has been written with Node.js in mind, trying to use the node-fetch instead of the readily available fetch in the browser.

The ideal way to break that chain is to offer an ESM edition of the @inrupt/solid-client-authn-browser so that bundlers won't try to pick the CJS / Node version of transitives.

Use Cases

Trying to build a basic Vue.js app with Vite instead of Webpack. The Vue community has been slowly backing away from Webpack, instead putting their effort into Vite (ESBuild for dev, Rollup for prod builds). Unfortunately, without hacking into node_modules with patch-package I was unable to create a working production build.

Additional information

As a workaround I explicitly list most of the transitive dependencies of @inrupt/solid-client-authn-browser and use a customResolver that's forced to pick ESM versions

As a workaround / hack I patched up the @inrupt/solid-client-authn-core package locally, to expose its ESM build even when require is in use. Mostly to avoid using the node-fetch that fails in the browser. However ideally, I hoped that the bundler can pick a browser / ESM targeted format of the packages.

Note that a fix for this would likely close #1815 as well. Also note that 2 more adjustments seemed to be necessary so that the project builds: https://gitlab.com/riovir/babalog/-/blob/main/vite.config.ts#L20-21

[jeswr]

Thanks for reporting - we will look into this.

Then, when processing the transitive dependencies, eventually imports the @inrupt/solid-client-authn-core however, picks the CommonJS edition. This edition, has been written with Node.js in mind, trying to use the node-fetch instead of the readily available fetch in the browser.

I'm surprised this is happening; note that @inrupt/solid-client-authn-core and @inrupt/solid-client-authn-browser both use @inrupt/universal-fetch under the hood which should only resolve to node-fetch when targeting node environments (with versions of node lower than 16.8 - see https://github.com/inrupt/universal-fetch/blob/main/src/index.ts).

What error were you getting that indicated this was happening and/or do you have a minimal repro?

Also note that 2 more adjustments seemed to be necessary so that the project builds

The stream patch is due to this upstream issue nodejs/readable-stream#516 (which we have proposed a patch for in nodejs/readable-stream#520).

What error were you getting that required the 'util': 'util/', entry?

[riovir]

What error were you getting that indicated this was happening and/or do you have a minimal repro?

Without the patch hack, ESBuild (development mode of Vite) works correctly. (Cool, let's deploy!) Then to my surprise, the prod build was unable to log me into my pod provider. Instead, the page just seemingly reloaded and nothing happened. So I grabbed a console.log and dove head-first into node_modules. Turned out that the CJS edition of the transitive dependencies was bundled in, and the universal-fetch trying to use node-fetch silently failed (a catch block swallowed the error). Therefore I was never able to reconstruct the Session. That prompted me to find a way to force ESM editions to the transitive packages required by @inrupt/solid-client-authn-browser. The reason this is not an issue with Webpack is that it was designed to consume CJS packages for the web.

both use @inrupt/universal-fetch under the hood which should only resolve to node-fetch when targeting node environments

The @inrupt/solid-client-authn-core is in fact correct. My hack messes with it as an innocent bystander. What really happens is as I understand it that @inrupt/solid-client-authn-browser lacking a "type": "module" (and a .mjs extension) in its package json, signals to Vite (Rollup in fact, as it happens during the prod build) that it .js extensions mean that they are in CommonJS. Then Vite/Rollup will pick the CJS editions of transitive dependencies even though it'd prefer ESM. The assumption of @inrupt/solid-client-authn-core is that when its CJS edition is loaded it's done for the server. However, @inrupt/solid-client-authn-browser's config causes the bundler to resolve CJS for the code intended for the browser.

What error were you getting that required the 'util': 'util/', entry?

With the original workaround, I got the following warning without it:

chunk-AC2VUBZ6.js?v=59b0ec33:11 Module "util" has been externalized for browser compatibility. Cannot access "util.inspect" in client code. See http://vitejs.dev/guide/troubleshooting.html#module-externalized-for-browser-compatibility for more details.

This is merely a symptom of the bundler picking the CJS edition of the package intended for the server. The new workaround indeed makes it unnecessary.

[riovir]

Update: by explicitly listing the dependencies of @inrupt/solid-client-authn-browser and forcing the bundler to pick the ESM versions of them the build also works, and the 'util': 'util/' entry is indeed unnecessary.

That said, this needs quite some dive into the Vite / Rollup config, plus an understanding of why the bundler gets confused by the CJS format of the browser library. An ESM build would be a big step toward a happy out-of-the-box experience.

[riovir]

I've created a small reproduction on Codesandbox: https://codesandbox.io/p/sandbox/vite-forked-vs5l4c

[riovir]

I also noticed that it's not CJS per se that causes the issue in the @inrupt/solid-client-authn-core but the use of fetch via a Node polyfill. However, to my knowledge, the fetch() has been stable since Node 18, so it may be worthwhile to consider dropping the universal-fetch / node-fetch transitive and favoring the built-in fetch() or the current platform. (Depending on how far back Inrupt supports Node)

The only security-supported Node version that doesn't have native fetch() support is Node 16 LTS, until 11 Sep 2023.

[elf-pavlik]

We encountered a problem using @inrupt/solid-client-authn-node with node 16, pinning node to 18 solved the problem for us so we didn't investigate further. I recall an error being thrown somewhere inside of @inrupt/solid-client-authn-core so it could be related. I could try to reproduce it and report it properly.

[jeswr]

SWhat universal fetch does is:

• On Node 18 and above the global fetch is used
• On Node ^16.8 and Node 17; the undici package is imported and used
• On other versions of Node node-fetch is used

An error report would be useful @elf-pavlik as I am not expecting such a change in behavior between Node 16 and Node 18.

[elf-pavlik]

reported in #3004 and fails in the undici package you mentioned above

[riovir]

Appreciate the update! I could remove a big chunk of Vitest code to make sure the @inrupt/solid-client-authn-browser works.
In fact, I have zero lines in my vitest.config.ts related to SOLID. I'm a happy developer.