[bugfix] Update PropTypes import statements to use wildcard syntax

[morozow]

Switched from default imports to wildcard imports for PropTypes across multiple files. This change ensures consistency in PropTypes usage and can help with tree-shaking during the build process.

Replaced all occurrences of import PropTypes from 'prop-types'; with import * as PropTypes from 'prop-types'; in the entire Material UI codebase.

Bug Report: [Error] Incorrect Import of PropTypes Causes Compilation Error

• I have followed (at least) the PR section of the contributing guide.

[siriwatknp]

@morozow Thanks for submitting the PR! @Janpot What do you think about the proposed fix?

[Janpot]

Trying the following in Node.js (node index.mjs):

// test.mjs
import * as PropTypes from 'prop-types';
// import PropTypes from 'prop-types';

console.log(PropTypes.any);

This prints undefined. The prop-types package only exports commonjs.

🤔 It's a bit strange, but these types don't look 100% correct to me. I'm seeing this change a long time ago. I'm wondering why they did that.
The package follows a similar structure wrt to its exports as react, yet react types are declared with export =. Which feels correct to me.

I'm a bit worried making this change because I believe it could result in broken ESM once we move to #43264

[morozow]

@Janpot I cloned your forked repository https://github.com/Janpot/material-ui, checked out the esm-exports branch. After running npm run build from the repository’s root, a repo can not be built now. Looks like my PR can't play in your sandbox right now. Give me a shout when your PR is set up and can build the whole app. I'll jump to repo then and see if my PR and your PR can agree

Note: the current PR is functioning well when integrated into the main branch.

[Janpot]

I cloned your forked repository https://github.com/Janpot/material-ui, checked out the esm-exports branch. After running npm run build from the repository’s root, a repo can not be built now.

We use pnpm install and the command is pnpm release:build. The branch definitely builds as you can see from our CI: https://app.circleci.com/pipelines/github/mui/material-ui/138049/workflows/5fec5701-c382-4e1c-a938-a3b6a10803dc

But you can probably spare yourself the effort, try running the following script in Node.js:

// ./test.mjs
import * as PropTypes1 from 'prop-types';
import PropTypes2 from 'prop-types';
import * as React1 from 'react';
import React2 from 'react';

console.log(!!PropTypes1.any, !!PropTypes2.any);
// false true
console.log(!!React1.useState, !!React2.useState);
// true true

I believe import * will only work when there is also esModuleInterop: true available or an equivalent in a bundler.

[morozow]

@Janpot As a result, the build was successful. I must mention that I haven’t had much time to fully set up a Material UI application from the PR branch locally, including configuring all dependencies and executing the pnpm release:build localy into node_modules and others.

Nevertheless, I see no reason to reject this PR at this point now. Could you perhaps provide a guide on how to quickly install the @mui/material fork repo branch along with all its dependencies built locally as an package.json dependencies item?
Just a note: the default method requires many dependencies internally source changes and others, like "@mui/material": "morozow/material-ui#bugfix/esm-exports-prop-types",

I’ve merged your esm-exports branch into my fork merged branches, and it built without any issues.

[morozow]

@Janpot Again my 2 cents, as esModuleInterop: true setting in TS configuration is a more advanced solution that aligns with modern JavaScript standards, facilitating the use of both ES and CommonJS modules seamlessly. It’s important to note that the tsconfig.json at the root of the project dictates how TypeScript handles .ts files, influencing module resolution and type definitions across the project.

Adopting esModuleInterop: true can lead to fewer issues when integrating with diverse package ecosystems.

Nevertheless, for supporting existing users who might rely on older configurations, consider employing versioning strategies. This way, newer versions can default to esModuleInterop: true, while older versions remain available to ensure backward compatibility without disrupting dependent projects.

[morozow]

@Janpot While retesting locally with the TypeScript compiler and tsconfig.json set to "esModuleInterop": false, your examples function correctly as console.log(!!PropTypes1.any); > returns true with import * as PropTypes1 from 'prop-types';. I understand that every project has its unique philosophy from inception to the present, but using a root tsc config by default everything works well across different compilers.
I’ve tested various methods, specifically on your branch also, including setting "esModuleInterop": false,. Everything seems to be working well.

Webpack is a different scenario, yet even here approaches like outputModule (which confessedly carries its own risks), libraryTarget: umd or commonjs2 and @babel/plugin-syntax-dynamic-import manage to get the job done.

Does this fit with our current solution?

Just to note: as of now, Material UI v6.0.2 cannot be built with tsc

[Janpot]

For #43264 the build command is

MUI_USE_PACKAGE_EXPORTS=true pnpm release:build

While working on this, the new mode is activated with a MUI_USE_PACKAGE_EXPORTS env var.

This is going to build a new file e.g. packages/mui-material/build/Button/Button.mjs which will be an ES module. This one will be loaded natively in node.js when you run node test.mjs with

// test.mjs
import Button from '@mui/material/Button';

console.log(Button.propTypes);

When you build from import * as PropTypes from 'prop-types'; this will produce an error

file:///Users/janpotoms/Projects/material-ui/packages/mui-material/build/Button/Button.mjs:399
  color: PropTypes /* @typescript-to-proptypes-ignore */.oneOfType([PropTypes.oneOf(['inherit', 'primary', 'secondary', 'success', 'error', 'info', 'warning']), PropTypes.string]),
                                                                              ^

TypeError: PropTypes.oneOf is not a function
    at file:///Users/janpotoms/Projects/material-ui/packages/mui-material/build/Button/Button.mjs:399:79
    at ModuleJob.run (node:internal/modules/esm/module_job:195:25)
    at async ModuleLoader.import (node:internal/modules/esm/loader:337:24)
    at async loadESM (node:internal/process/esm_loader:34:7)
    at async handleMainPromise (node:internal/modules/run_main:106:12)

When you build from import * as PropTypes from 'prop-types'; this will print the proptypes

When you run the commonjs variant, node test.cjs with

const Button = require('@mui/material/Button').default;

console.log(Button.propTypes);

this will also just print the proptypes (because babel added an interopRequireDefault helper)

To test this within the material-ui repo, you can temporarily add publishConfig.linkDirectory: true to the packages/mui-material/package.json and run pnpm install.

This is the hard way of reproducing the problem. My little snippet from above essentially replicates what's happening inside of @mui/material after we make them strict ESM.

It'll be important to be able to load @mui/material natively in node.js because it'll be a requirement for pigment css to e.g. load the theme logic inside of vite.config.ts.

Nevertheless, I see no reason to reject this PR at this point now.

I hope the long explanation makes it clear that the changes are not mergeable as for now. I understand this is frustrating. I believe the best next steps we could take is investigate why named exports in react behave differently from named exports in prop-types as my script above demonstrates and fix the problem first in prop-types so that it also prints true true.

[Janpot]

@morozow I think I figured out the problem with prop-types. Node.js uses static analysis to find which names are exported from commonjs modules.
See https://nodejs.org/api/esm.html#commonjs-namespaces and https://github.com/nodejs/cjs-module-lexer/tree/1.2.2
react is statically analyzable so it provides named exports. prop-types is not and therefore it only finds a default export. The prop-types repo is archived since last week, we can't fix this on their end. The only reliable solution that I can think of is to keep with import PropTypes from 'prop-types' for now.

[morozow]

@Janpot You've highlighted some valuable ideas. I've tailored the solution to directly address the core issue with the tsc in the upcoming PR #43736.

We can either keep this current PR open for future reference or close it, depending on your preference. However, we should consider enhancing a codebase with a new PR by incorporating utilities to enable tsc

[morozow]

The problem has been fixed by updating the documentation: #43747