lidofinance / ui

React UI Components for Lido projects.
MIT License
20 stars 16 forks source link

Fix ESM build #460

Closed alx-khramov closed 11 months ago

alx-khramov commented 11 months ago

Problem

Current ESM build of lido-ui is not usable. This PR is trying to fix it.

Why not usable?

How it was discovered

Recently, I've added lido-ui as a dependency to reef-knot because I needed some UI components. reef-knot is a pure ESM module, because it is a wagmi requirement. So, in the compiled dist of reef-knot there are ESM imports of lido-ui. lido-ui is used on our Ethereum Staking Widget. If you build it and check the resulting code, you will see, that lido-ui is used as a CJS module via CJS require. The reason for this is misconfigured package.json (see "Why not usable?"). When I added updated reef-knot with lido-ui ESM imports to the widget, the build became broken with such error:

.../ethereum-staking-widget/node_modules/@reef-knot/connect-wallet-modal/dist/components/Ledger/LedgerConnectionScreen.js:2
import { Stack, StackItem, useBreakpoint, Text } from '@lidofinance/lido-ui';
         ^^^^^
SyntaxError: Named export 'Stack' not found. The requested module '@lidofinance/lido-ui' is a CommonJS module, which may not support all module.exports as named exports.

You can switch to this commit and see.

How I tried to solve it

  1. Added required fields to package.json
  2. Tried to configure swc to build code with full paths in imports, but no luck.
  3. Manually updated all imports in the source code to conform ESM requirements.
  4. Configured eslint to warn about using imports without extensions
  5. Renamed some config files written in CJS to *.cjs, so the interpreter could understand what it is
  6. Configured jest to work with ESM

Problem with warnings about imports without extensions

Since I wasn't able to configure swc to build code with explicit imports, we want to be sure that all developers of lido-ui will use ESM-compatible imports in the source code, therefore we need some kind of linter for that. It would be ideal to set "moduleResolution": "nodenext" parameter in tsconfig (docs). In that case, TS interpreter checks that imports are correct, and, for example, WebStorm IDE highlights the wrong imports. But this setting changes module resolution rules and the build breaks with a lot of errors, including type errors. I don't want to dive into attempts to solve this now. So, I tried to configure eslint for this purpose. It turned out, that there is no eslint rule for this case. There is eslint-plugin-import, which "is built assuming you're using babel, not native ES Modules". And there is node/file-extension-in-import rule, which would work if we were using vanilla js, but it requires using '.ts' extensions for ts files, which is wrong.
However, the combination of these rules works in a hacky way: https://github.com/lidofinance/ui/pull/460/commits/265d46d89632445e3c61d32af9f4c275c77c1c41#diff-e1ce717e1179a0db14e90ec5374768a206651ca807db58352991eb7895ed7c9cR39-R43 One rule requires using filename with extension in import, another rules requires using .js instead of .ts. But it doesn't work for imports like this:

import { foo } from '..'
import { foo } from '.'

A developer can forget to add filename with extension in this case, the build will not fail, but the released version will break Ethereum Staking Widget (for example) build with ERR_UNSUPPORTED_DIR_IMPORT error.

Styled Components Issue

This is a serious blocker. styled-components v5 has issue with ESM:

This issue is fixed in v6 by adding styled as a named export instead of default export.

Solution 1: wrapper (UPD: this PR contains this solution)

I faced it when reconfigured reef-knot to work with ESM. But v6 was in beta then and I decided to use a simple wrapper for styled-components. This wrapper is used like this:

import styled, { css, keyframes } from '../../utils/styledWrapper.js';

It was okay, because I was the only developer of reef-knot and I remember that I must use this wrapper instead of importing directly from the package. Also, there are not so much UI components in reef-knot. But it looks bad for lido-ui.

Solution 2: update styled-components to v6

Looks like it will solve the issue. The migration guide looks not so scary: https://styled-components.com/docs/faqs#what-do-i-need-to-do-to-migrate-to-v6 But lido-ui uses styled-components as a peer dependency, so updating it in lido-ui also requires updating it everywhere, which looks like a lot of work.