Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
1 task done
jollytoad opened this issue May 11, 2021 · 13 comments
Open
1 task done

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

jollytoad opened this issue May 11, 2021 · 13 comments
Labels
new feature New feature or request

Comments

@jollytoad
Copy link

  • I have searched the issues of this repository and believe that this is not a duplicate.

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 jollytoad added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label May 11, 2021
@jollytoad
Copy link
Author

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

@eps1lon
Copy link
Member

eps1lon commented May 12, 2021

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
Copy link
Member

This makes me think of a past exploration I had to modernize our CDN /examples. See skypackjs/skypack-cdn#118.

@oliviertassinari
Copy link
Member

oliviertassinari commented May 30, 2021

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.

@oliviertassinari oliviertassinari added core Infrastructure work going on behind the scenes and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jun 3, 2021
@oliviertassinari oliviertassinari changed the title Add exports to package.json to support CDNs such as JSPM [core] Add exports to package.json Jun 3, 2021
@oliviertassinari oliviertassinari changed the title [core] Add exports to package.json [core] Add exports to the package.json Jun 3, 2021
@jollytoad
Copy link
Author

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
Copy link
Member

eps1lon commented Jul 29, 2021

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: #26264 (comment)

@thepian
Copy link

thepian commented Jan 2, 2023

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 oliviertassinari added new feature New feature or request and removed core Infrastructure work going on behind the scenes labels Jan 2, 2023
@oliviertassinari
Copy link
Member

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
Copy link
Author

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

@guybedford
Copy link

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
Copy link
Member

@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
Copy link

guybedford commented Jan 15, 2023

@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.

@oliviertassinari oliviertassinari changed the title [core] Add exports to the package.json [ESM][core] Add exports to the package.json Aug 12, 2023
@jedwards1211
Copy link
Contributor

jedwards1211 commented Oct 23, 2023

Using an export map might play more nicely with Next.js...I just debugged this issue:
vercel/next.js#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants