[code-infra] Prevent wrong nested imports in Base UI

[oliviertassinari]

Reflect mui/base-ui#488 and mui/base-ui#805.

[mui-bot]

Netlify deploy preview

https://deploy-preview-44426--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against f2f1986

[michaldudak]

Once mui/base-ui#821 is merged, importing private utilities won't be possible, so these rules will become obsolete.

[oliviertassinari]

@michaldudak I'm not seeing this. Not being possible to import files that are not in the exports field of package.json !== having the exports field of package.json to follow the convention of only one level deep export.

I guess TypeScript would statically report when imports are wrong, replacing eslint there.

[michaldudak]

Could you explain what you mean? With the exports field, we won't need this convention anymore. Whatever is exported is available to use. Everything else is impossible to import.

[oliviertassinari]

@michaldudak but this won't prevent us to make:

import bar from '@base-ui-components/react/tree-view/bar';

a valid import. I assume that we don't want to allow anything else other than one-level deep imports. Or at least, not without the pain of disabling the rule in specific instances.

---

There were discussions related to this at mui/mui-x#10920. Maybe we should approach this like this:

1. Untill we only have package.json.exports, we make it harder to support:

import { frFR } from '@mui/x-date-pickers/locales/frFR';

2. Once we can guarantee that private modules can never be imported then we fully embrace:

import { frFR } from '@mui/x-date-pickers/locales/frFR';

because it's likely a better DX than:

import { frFR } from '@mui/x-date-pickers/locales-frFR';

[michaldudak]

@michaldudak but this won't prevent us to make:
import bar from '@base-ui-components/react/tree-view/bar';

It will. If only ./tree-view is present in package.json exports, nothing else from it will be possible to be imported.

I created a project for testing the new package layout with the exports field: https://github.com/michaldudak/base-ui-bundler-tests. It uses a build from mui/base-ui#821.
Having exports defined as such:

"exports": {
    ".": {
      "require": {
        "types": "./cjs/index.d.ts",
        "default": "./cjs/index.js"
      },
      "import": {
        "types": "./esm/index.d.ts",
        "default": "./esm/index.js"
      }
    },
    "./*": {
      "require": {
        "types": "./cjs/*/index.d.ts",
        "default": "./cjs/*/index.js"
      },
      "import": {
        "types": "./esm/*/index.d.ts",
        "default": "./esm/*/index.js"
      }
    }
  }

Makes only top-level identifiers importable from other projects. So for example this fails:

import { useMenuItem } from '@base-ui-components/react/Menu/Item/useMenuItem'

[oliviertassinari]

@michaldudak Agree with this.

I was probably not clear enough, I think the discussion is about the value of having fail safes, i.e. what happens if someone go to the package.json and manually add a to the exports field a deep .js module? This eslint rule can catch it.

[michaldudak]

If someone makes the effort to include the deeply nested identifier in exports, there's likely a good reason to do so. I would treat the exports field as the source of truth for what's public, not an ESLint rule (besides, the ESLint rule would catch the violation at call site, not in package.json, so, for example, Material UI developers would see a problem when trying to import something that was explicitly exported from Base UI.

[oliviertassinari]

If someone makes the effort to include the deeply nested identifier in exports

@michaldudak We have to have all exports in exports now, why is it more effort to add one nested than one flat?

ESLint rule would catch the violation at call site, not in package.json, so, for example, Material UI developers would see a problem when trying to import something that was explicitly exported from Base UI.

I'm missing something: If an npm package exports a module, in the same repository, a docs or test should exists for it, given that we don't allow relative imports for tests and docs, the eslint rule would catch it in the same PR, so it works?

[michaldudak]

I fail to see why we need a convention when we have a proper tool for making identifiers public.

@michaldudak We have to have all exports in exports now, why is it more effort to add one nested than one flat?

It's not necessarily more effort, but it has to be done intentionally.