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

[Error] Incorrect Import of PropTypes Causes Compilation Error #43700

Closed
morozow opened this issue Sep 10, 2024 · 17 comments · Fixed by #43747
Closed

[Error] Incorrect Import of PropTypes Causes Compilation Error #43700

morozow opened this issue Sep 10, 2024 · 17 comments · Fixed by #43747
Labels
external dependency Blocked by external dependency, we can’t do anything about it package: material-ui Specific to @mui/material typescript

Comments

@morozow
Copy link
Contributor

morozow commented Sep 10, 2024

Steps to reproduce

Link to live example: local dev as there's no TS completion to build live phase

Steps:

  1. Set up a Material UI project 6.0.2 with TypeScript.
  2. Use a Material UI component that imports PropTypes with import PropTypes from 'prop-types';.
  3. Run the TypeScript compiler.

Current behavior

When using Material UI with TypeScript, I encountered a compilation error due to the incorrect use of import PropTypes from 'prop-types'; in the codebase.

Typescript Compiler error:

node_modules/@mui/utils/chainPropTypes/chainPropTypes.d.ts:1:8 - error TS1192: Module '"path/to/project/node_modules/@types/prop-types/index"' has no default export.

1 import PropTypes from 'prop-types';

Current example of Material UI implementation:

Since the prop-types library does not have a default export, this import statement is incorrect and should instead use named imports to align with ES6 module standards.
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/prop-types/index.d.ts

Solution: apply import * as PropTypes from 'prop-types';

Expected behavior

The project should compile without errors.

Context

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

Additional Context

  • Node.js version: v20.9.0
  • yarn version: 1.22.19
  • TypeScript version: 5.6.2
  • MUI System version: 6.0.2

Your environment

npx @mui/envinfo
  System:
    OS: macOS 14.1.1
  Binaries:
    Node: 20.9.0 - /usr/local/bin/node
    npm: 10.2.4 - /usr/local/bin/npm
    pnpm: Not Found
  Browsers:
    Chrome: 128.0.6613.120
    Edge: Not Found
    Safari: 17.1
  npmPackages:
    @emotion/react: ^11.13.3 => 11.13.3 
    @emotion/styled: ^11.13.0 => 11.13.0 
    @mui/core-downloads-tracker:  6.0.2 
    @mui/envinfo: ^2.0.25 => 2.0.25 
    @mui/icons-material: ^6.0.2 => 6.0.2 
    @mui/material: ^6.0.2 => 6.0.2 
    @mui/private-theming:  6.0.2 
    @mui/styled-engine:  6.0.2 
    @mui/styles: ^6.0.2 => 6.0.2 
    @mui/system:  6.0.2 
    @mui/types:  7.2.16 
    @mui/utils:  6.0.2 
    @mui/x-date-pickers: ^7.16.0 => 7.16.0 
    @mui/x-internals:  7.16.0 
    @types/react: ^18.3.5 => 18.3.5 
    react: ^18.3.1 => 18.3.1 
    react-dom: ^18.3.1 => 18.3.1 
    typescript: ^5.6.2 => 5.6.2 

Search keywords: error, types, d.ts

@morozow morozow added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Sep 10, 2024
@morozow
Copy link
Contributor Author

morozow commented Sep 10, 2024

Nitpick: If anyone encounters this issue, here is a quick fix for Unix systems using Yarn post-install:

#!/bin/bash
cd ./

echo "Tuning of TypeScript types for React node_modules packages..."

# Find all .ts files in the project directory including node_modules and replace the import statement
find . -type f -name "*.ts" -exec sed -i '' 's/import PropTypes from '"'"'prop-types'"'"';/import * as PropTypes from '"'"'prop-types'"'"';/g' {} +

# Find all .ts files in the project directory including node_modules and replace the import statement
find . -type f -name "*.ts" -exec sed -i'' -e 's#import React[^;]*#import * as React from '"'"'react'"'"'#g' {} +

Smth like this into package.json:

...
"scripts": {
    "postinstall": "bash ./bin/post.install.sh",
}
...

@morozow
Copy link
Contributor Author

morozow commented Sep 10, 2024

And to add my 2 cents, given that we’re facing this issue as the optimal fix appears to involve the Unix sed command, pointing to a deployment issue with Material UI. And just take a note, Bitbucket and other deployment environments by git lack access to the Unix sed command. Specifically, my process involves locally building a React application that includes Material UI, then pushing this build to a Git repository. Following this, the deployment can be executed to CloudFront using bitbucket-pipelines.yml as one option, or it can also be done locally through a simple bash script. However, using the local script option means missing out on collaborative team challenges. Could we prioritise resolving this issue as soon as possible please?

Here's a sample Bash script for deploying a React application that uses Material UI:

#!/bin/bash
cd ./

echo "Deploying Material UI Application to CloudFront..."

# IMPORTANT: before deploy you MUST/CAN clear all S3 Bucket files then to deploy, by the other case you can meet a conflict or S3 Bucket potential overload

aws s3 sync ./build s3://$CLOUDFRONT_DISTRIBUTION_S3_BUCKET_NAME/
aws cloudfront create-invalidation --distribution-id $CLOUDFRONT_DISTRIBUTION_ID --paths "/*" | grep Id

echo "CloudFront invalidation is going..."
exit 0

morozow added a commit to morozow/material-ui that referenced this issue Sep 11, 2024
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.

Bug Report: mui#43700
@morozow
Copy link
Contributor Author

morozow commented Sep 11, 2024

I have verified that each package includes "@types/prop-types": "^15.7.12", which indicates that the entire project can transition to the correct PropTypes import syntax for TypeScript compatibility.

Here is the Pull Request addressing this fix: #43701

Please review it and let me know if there are any additional contributions or changes needed.

@siriwatknp siriwatknp added external dependency Blocked by external dependency, we can’t do anything about it package: material-ui Specific to @mui/material typescript and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Sep 11, 2024
@oliviertassinari
Copy link
Member

I changed those imports in #23116, maybe it was wrong 🤔

@morozow
Copy link
Contributor Author

morozow commented Sep 12, 2024

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

We can either keep this #43701 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
Copy link
Contributor Author

morozow commented Sep 12, 2024

@oliviertassinari The issue with tsc primarily pertains to *.d.ts into utils files. From what I can tell, your enhancement doesn’t seem to break the TypeScript behaviour

@Janpot
Copy link
Member

Janpot commented Sep 12, 2024

I changed those imports in #23116, maybe it was wrong 🤔

I think you were correct. I believe @types/prop-types is wrong.

@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 12, 2024

@Janpot Yeah agree, the problem is not with Material UI.

In https://unpkg.com/browse/prop-types@15.8.1/index.js, I see: module.exports = . So it should be imported like this on Node.js:

const propTypes1 = require('prop-types');

import propTypes2 from 'prop-types';
import * as propTypes3 from 'prop-types';

propTypes1. // valid
propTypes2. // valid
propTypes3.default. // valid

let's fix @types/prop-types instead: DefinitelyTyped/DefinitelyTyped#70539.

@morozow
Copy link
Contributor Author

morozow commented Sep 12, 2024

@Janpot @oliviertassinari I might be mistaken, but it appears that we should refer to the @types/prop-types package: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/prop-types/index.d.ts to address the concerns raised in my pull requests like #43736

@morozow
Copy link
Contributor Author

morozow commented Sep 13, 2024

@oliviertassinari It's impressive how smoothly the previous decision was altered by #43700 (comment). Just a quick observation: the updated PR description still seems incorrect after update, as it references a *.ts file instead of the needed @types/prop-types options. I was happy to assist. I look forward to the next release, as the current issue halts all deliveries. Thank you.

@Janpot
Copy link
Member

Janpot commented Sep 13, 2024

@morozow We appreciate your willingness to contribute, but to reduce the risk of contributions being rejected, it would be best to wait until we've agreed on the appropriate solution. To be fully honest, I still don't understand what the exact problem is. At this point, the best thing you could do is to share a real reproduction (e.g. a codesandbox, or a github repository) that clearly demonstrates what you're trying to do and the error you're running into so that we can take a look. These imports have been there in this form since 2020, without any issues, I hope you understand that we're not going to make hasty changes to to them, especially when it's demonstrated they will be harmful in the future.

It's clear that there is a mismatch between what the prop-types package exports and what the @types/prop-types declares. You may have to adjust your typescript configuration to get it compiled (see allowSyntheticDefaultImports).

@morozow
Copy link
Contributor Author

morozow commented Sep 13, 2024

@Janpot I've set up a fresh repository at https://github.com/morozow/material-ui-tsc-fix.
I started by adding a default tsconfig.json following the minimal configuration guidelines from https://mui.com/material-ui/guides/typescript/#minimum-configuration.
Next, I installed Material UI as instructed at https://mui.com/material-ui/getting-started/installation/ by npm.
After that, I created a default example using the instructions from https://mui.com/material-ui/guides/typescript/#complications-with-the-component-prop.
Upon running tsc, I observe the following:

user@machine material-ui-tsc-fix % tsc
node_modules/@mui/system/borders/index.d.ts:1:10 - error TS2305: Module '"./borders"' has no exported member 'default'.

1 export { default } from './borders';
           ~~~~~~~

node_modules/@mui/system/cssGrid/index.d.ts:1:10 - error TS2305: Module '"./cssGrid"' has no exported member 'default'.

1 export { default } from './cssGrid';
           ~~~~~~~

node_modules/@mui/system/display/index.d.ts:1:10 - error TS2305: Module '"./display"' has no exported member 'default'.

1 export { default } from './display';
           ~~~~~~~

node_modules/@mui/system/flexbox/index.d.ts:1:10 - error TS2305: Module '"./flexbox"' has no exported member 'default'.

1 export { default } from './flexbox';
           ~~~~~~~

node_modules/@mui/system/palette/index.d.ts:1:10 - error TS2305: Module '"./palette"' has no exported member 'default'.

1 export { default } from './palette';
           ~~~~~~~

node_modules/@mui/system/positions/index.d.ts:1:10 - error TS2305: Module '"./positions"' has no exported member 'default'.

1 export { default } from './positions';
           ~~~~~~~

node_modules/@mui/system/shadows/index.d.ts:1:10 - error TS2305: Module '"./shadows"' has no exported member 'default'.

1 export { default } from './shadows';
           ~~~~~~~

node_modules/@mui/system/sizing/index.d.ts:1:10 - error TS2305: Module '"./sizing"' has no exported member 'default'.

1 export { default } from './sizing';
           ~~~~~~~

node_modules/@mui/system/spacing/index.d.ts:1:10 - error TS2305: Module '"./spacing"' has no exported member 'default'.

1 export { default } from './spacing';
           ~~~~~~~

node_modules/@mui/system/style/index.d.ts:1:10 - error TS2305: Module '"./style"' has no exported member 'default'.

1 export { default } from './style';
           ~~~~~~~

node_modules/@mui/system/typography/index.d.ts:1:10 - error TS2305: Module '"./typography"' has no exported member 'default'.

1 export { default } from './typography';
           ~~~~~~~

node_modules/@mui/utils/chainPropTypes/chainPropTypes.d.ts:1:8 - error TS1192: Module '"/path/to/sandbox/material-ui/material-ui-tsc-fix/node_modules/@types/prop-types/index"' has no default export.

1 import PropTypes from 'prop-types';
         ~~~~~~~~~

node_modules/@mui/utils/elementAcceptingRef/elementAcceptingRef.d.ts:1:8 - error TS1192: Module '"/path/to/sandbox/material-ui/material-ui-tsc-fix/node_modules/@types/prop-types/index"' has no default export.

1 import PropTypes from 'prop-types';
         ~~~~~~~~~

node_modules/@mui/utils/elementTypeAcceptingRef/elementTypeAcceptingRef.d.ts:1:8 - error TS1192: Module '"/path/to/sandbox/material-ui/material-ui-tsc-fix/node_modules/@types/prop-types/index"' has no default export.

1 import PropTypes from 'prop-types';
         ~~~~~~~~~

node_modules/@mui/utils/integerPropType/integerPropType.d.ts:1:8 - error TS1192: Module '"/path/to/sandbox/material-ui/material-ui-tsc-fix/node_modules/@types/prop-types/index"' has no default export.

1 import PropTypes from 'prop-types';
         ~~~~~~~~~

node_modules/@mui/utils/refType/refType.d.ts:1:8 - error TS1192: Module '"/path/to/sandbox/material-ui/material-ui-tsc-fix/node_modules/@types/prop-types/index"' has no default export.

1 import PropTypes from 'prop-types';
         ~~~~~~~~~


Found 16 errors in 16 files.

Errors  Files
     1  node_modules/@mui/system/borders/index.d.ts:1
     1  node_modules/@mui/system/cssGrid/index.d.ts:1
     1  node_modules/@mui/system/display/index.d.ts:1
     1  node_modules/@mui/system/flexbox/index.d.ts:1
     1  node_modules/@mui/system/palette/index.d.ts:1
     1  node_modules/@mui/system/positions/index.d.ts:1
     1  node_modules/@mui/system/shadows/index.d.ts:1
     1  node_modules/@mui/system/sizing/index.d.ts:1
     1  node_modules/@mui/system/spacing/index.d.ts:1
     1  node_modules/@mui/system/style/index.d.ts:1
     1  node_modules/@mui/system/typography/index.d.ts:1
     1  node_modules/@mui/utils/chainPropTypes/chainPropTypes.d.ts:1
     1  node_modules/@mui/utils/elementAcceptingRef/elementAcceptingRef.d.ts:1
     1  node_modules/@mui/utils/elementTypeAcceptingRef/elementTypeAcceptingRef.d.ts:1
     1  node_modules/@mui/utils/integerPropType/integerPropType.d.ts:1
     1  node_modules/@mui/utils/refType/refType.d.ts:1
  ~~~~~~~~~~~~~~~~~~~

Consequently, I encounter an issues documented at #43700 & #43685

@Janpot
Copy link
Member

Janpot commented Sep 13, 2024

Assuming a bundler will be consuming your library, you should be able to set "allowSyntheticDefaultImports": true in your tsconfig.json. We can work towards removing this setting in our tsconfig files to improve our internal import behavior, but at the moment we won't fully be able to as we have the previously demonstrated prop-types mismatch with its typings package.

@morozow
Copy link
Contributor Author

morozow commented Sep 13, 2024

@Janpot I’m pleased that it’s working. If there’s a need for documentation improvement, here’s the PR: #43747
Thank you!

@Janpot
Copy link
Member

Janpot commented Sep 16, 2024

@morozow I'm glad it works for you. Thank you for improving the docs. Is this issue sufficiently addressed for you so that we can close this ticket and the related ones?

We will have to revise our imports as part of #43264. I'm going to improve/fix the node-esm runtime tests before we touch those imports.

@Janpot Janpot added the status: waiting for author Issue with insufficient information label Sep 16, 2024
@morozow
Copy link
Contributor Author

morozow commented Sep 16, 2024

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

@github-actions github-actions bot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer and removed status: waiting for author Issue with insufficient information labels Sep 16, 2024
@Janpot Janpot closed this as completed Sep 16, 2024
Copy link

This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue.
Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

Note

We value your feedback @morozow! How was your experience with our support team?
If you could spare a moment, we'd love to hear your thoughts in this brief Support Satisfaction survey. Your insights help us improve!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external dependency Blocked by external dependency, we can’t do anything about it package: material-ui Specific to @mui/material typescript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants