mui / material-ui

Material UI: Ready-to-use foundational React components, free forever. It includes Material UI, which implements Google's Material Design.
https://mui.com/material-ui/
MIT License
92.39k stars 31.82k forks source link

[ESM][core] Add `exports` to the package.json #26254

Open jollytoad opened 3 years ago

jollytoad commented 3 years ago

Summary 💡

An exports property in the package.json of @material-ui/icons (and other packages), to aid discovery of the officially exported modules. To support CDNs such as https://jspm.org/

Examples 🌈

"exports": { "./*": "./*.js" }

Motivation 🔦

I want to consume material-ui via the JSPM CDN, but due to a lack of exports, it has to work out for itself what modules should be exported, and some icons are missing.

jollytoad commented 3 years ago

This would be useful applied to both the 4.x branch and the upcoming 5.x too.

eps1lon commented 3 years ago

This would be useful applied to both the 4.x branch and the upcoming 5.x too.

4.x only receives critical updates. And since there's some possible breakage with regards to file extension usage we probably can't even apply it to 4.x. So 5.x is the safer target for now.

oliviertassinari commented 3 years ago

This makes me think of a past exploration I had to modernize our CDN /examples. See https://github.com/skypackjs/skypack-cdn/issues/118.

oliviertassinari commented 3 years ago

While I don't really buy in the motivation raised in the issue's description. The CDN like builder use case seems niche (you can prototype with codesandbox and build with vite/next.js, why a CDN?). Looking closer, it seems to prevent private paths to be imported. See:

This sounds really interesting. We have been struggling in the past with developers that import too deep and have a broken page.

jollytoad commented 2 years ago

Ok, finally got our app updated to MUI 5.0.0-beta.2 - would the core MUI team still be receptive to have exports added to the package.json files, so that the future CDN based systems (JSPM/Skypack etc) can make use of this to know what modules are officially exported from MUI packages?

eps1lon commented 2 years ago

would the core MUI team still be receptive to have exports added to the package.json files,

Nothing has changed about that. Be aware that a "exports": { "./*": "./*.js" } does not work for us: https://github.com/mui-org/material-ui/pull/26264#issuecomment-843095408

thepian commented 1 year ago

It seems that @mui are not a viable option for prototyping UIs in Framer. 18 months to pick your toes about what surely cannot be a big deal looks bad.

oliviertassinari commented 1 year ago

I want to consume material-ui via the JSPM CDN, but due to a lack of exports, it has to work out for itself what modules should be exported, and some icons are missing.

@jollytoad Is this a limitation of JSPM? I guess you could use https://esm.sh/ instead, e.g. https://codepen.io/oliviertassinari/pen/VwBjrvP?editors=1010

import React from "https://esm.sh/react@18"
import { Button } from "https://esm.sh/@mui/material"
import { createRoot } from "https://esm.sh/react-dom@18/client"

const App = () => {
  return (
    <div>
      <h1>Hello React! ⚛️</h1>
      <p>Merry Xmas 🎄</p>
      <Button variant="contained">
        Hello
      </Button>
    </div>
  )
}

const root = createRoot(document.getElementById("root"))
root.render(<App />)
Screenshot 2023-01-02 at 13 27 20

It seems that https://github.com/mui are not a viable option for prototyping UIs in Framer

@thepian It seems to work for me, following https://www.framer.com/developers/guides/using-external-code/:

import { Button } from "https://esm.sh/@mui/material?external=react"

export default function Test(props) {
    return <Button variant="contained">Hello</Button>
}
Screenshot 2023-01-02 at 13 38 22

https://month-induce-864585.framer.app/


Off-topic, it would be good to revamp https://github.com/mui/material-ui/tree/master/examples/cdn, using UMD files feels outdated with today's frontend infrastructure.

jollytoad commented 1 year ago

I'm no longer working on a project with MUI or JSPM any more. Maybe @guybedford could elaborate on using JSPM over ESM?

guybedford commented 1 year ago

Note for JSPM support, the exports overrides created by @jollytoad are still running at https://github.com/jspm/overrides/blob/main/overrides.json#L486. They may need updating for the latest material-ui which may be the issue here.

oliviertassinari commented 1 year ago

@guybedford Is there a way JSPM could make the need for an overrides.json obsolete? I don't understand why it would need these when esm.sh doesn't.

guybedford commented 1 year ago

@oliviertassinari JSPM is an optimizing CDN - and package optimization involves combining multiple modules into one. When that optimization is done, you want to combine together modules which are private that aren't going to be publicly imported, like pkg/internal/utils.js, otherwise you risk duplicating that code execution via both import('pkg') containing a bundled version of that file and import('pkg/internal/utils.js') containing another separate version of that file. Thus package optimization via module inlining requires a list of public entry points to not inline, and this is exactly what the "exports" field provides. When there is no "exports" field, the JSPM CDN will automatically detect this list based on statistics from analysing all modules on npm and how they import other packages. This statistical method is not completely accurate though, especially for newer packages or packages with lots of subpaths where users may not have imported everything before. The best fix is for all packages to use the "exports" field, but pending that when the automatic detection doesn't work (and it's usually pretty good) the overrides can provide a fallback approach. The override process explicitly states creating a PR like this one to upstream the exports as well, thus making the override service a necessity only in the case where this upstreaming gets blocked. esm.sh doesn't have this problem because it's not trying to solve global package optimization for the browser, and will quite happily serve whatever variations you request, regardless of internal resolution conflicts or duplications.

jedwards1211 commented 8 months ago

Using an export map might play more nicely with Next.js...I just debugged this issue: https://github.com/vercel/next.js/issues/57285

Basically they seem to have recently added this Webpack alias when static rendering:

  '@mui/material$': '/Users/andy/gh/next-mui-esm-issue/node_modules/@mui/material/index.js',

But, unfortunately, they also have mainFields: ['main', 'module'], meaning an import from ./Menu looks at ./Menu/package.json and resolves to its main field: ../node/Menu/index.js, which is a CJS module, when the index ES module was intending to import another ES module, yikes.

Next's config defaults seem pretty messed up to me (I remember seeing somewhere someone asked them to reverse to ['module', 'main'] but they said it caused too many things to break) but if @mui/material had an exports map in the root, I think this Next issue wouldn't have happened.