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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[code-infra] Move next config to ESM #40869

Merged
merged 7 commits into from
Feb 8, 2024
Merged

Conversation

Janpot
Copy link
Member

@Janpot Janpot commented Jan 31, 2024

We're having modules that are loaded both by the app and the config file (e.g. docs/config). This change allows us to author them in ESM for both environments. I'm having trouble loading the docs/config when moving it into @mui/docs

@Janpot Janpot added the docs Improvements or additions to the documentation label Jan 31, 2024
@mui-bot
Copy link

mui-bot commented Jan 31, 2024

Netlify deploy preview

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

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against f0731ea

@oliviertassinari oliviertassinari added scope: code-infra Specific to the core-infra product and removed docs Improvements or additions to the documentation labels Jan 31, 2024
@Janpot Janpot marked this pull request as ready for review February 2, 2024 08:05
@Janpot Janpot requested a review from a team February 2, 2024 08:06
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 6, 2024
Comment on lines +7 to +8
import withDocsInfra from './nextConfigDocsInfra.js';
import { findPages } from './src/modules/utils/find.mjs';
Copy link
Member

@oliviertassinari oliviertassinari Feb 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious about this. Why add the extensions?

Edit: oh lol really? https://nodejs.org/api/esm.html#mandatory-file-extensions

Mandatory file extensions

Copy link
Member Author

@Janpot Janpot Feb 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

node.js follows the native browser implementation in its resolver for esm, full paths mandatory. I guess it guarantees predictable amount of io access during module loading? It's more important on the web.
I think I saw somewhere that they're porting the classic node resolver to esm, but it's still experimental.

@github-actions github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels Feb 7, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 7, 2024
@Janpot Janpot enabled auto-merge (squash) February 7, 2024 11:07
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 7, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: code-infra Specific to the core-infra product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants