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.52k stars 31.89k forks source link

v4: issue with @types/jss #14608

Closed VincentLanglet closed 5 years ago

VincentLanglet commented 5 years ago

I have the error when I update to v4 for core and styles packages.

node_modules/@material-ui/core/styles/createGenerateClassName.d.ts:1:10 - error TS2305: Module '"../../../../../../../../../Users/vlanglet/Project/cloud/front/node_modules/jss/src"' has no exported member 'GenerateClassName'.

1 import { GenerateClassName } from 'jss';
           ~~~~~~~~~~~~~~~~~

node_modules/@material-ui/core/styles/jssPreset.d.ts:1:10 - error TS2724: Module '"../../../../../../../../../Users/vlanglet/Project/cloud/front/node_modules/jss/src"' has no exported member 'JSSOptions'. Did you mean 'JssOptions'?

1 import { JSSOptions } from 'jss';
           ~~~~~~~~~~

node_modules/@material-ui/core/styles/withStyles.d.ts:32:15 - error TS2694: Namespace '"/Users/vlanglet/Project/cloud/front/node_modules/jss/src/index"' has no exported member 'CreateStyleSheetOptions'.

32   extends JSS.CreateStyleSheetOptions<ClassKey> {

Expected Behavior 🤔

No error

Current Behavior 😯

node_modules/@material-ui/core/styles/createGenerateClassName.d.ts:1:10 - error TS2305: Module '"../../../../../../../../../Users/vlanglet/Project/cloud/front/node_modules/jss/src"' has no exported member 'GenerateClassName'.

1 import { GenerateClassName } from 'jss';
           ~~~~~~~~~~~~~~~~~

node_modules/@material-ui/core/styles/jssPreset.d.ts:1:10 - error TS2724: Module '"../../../../../../../../../Users/vlanglet/Project/cloud/front/node_modules/jss/src"' has no exported member 'JSSOptions'. Did you mean 'JssOptions'?

1 import { JSSOptions } from 'jss';
           ~~~~~~~~~~

node_modules/@material-ui/core/styles/withStyles.d.ts:32:15 - error TS2694: Namespace '"/Users/vlanglet/Project/cloud/front/node_modules/jss/src/index"' has no exported member 'CreateStyleSheetOptions'.

32   extends JSS.CreateStyleSheetOptions<ClassKey> {

Context 🔦

=> Found "jss@10.0.0-alpha.10"
info Reasons this module exists
   - "@material-ui#styles" depends on it
   - Hoisted from "@material-ui#styles#jss"
info Disk size without dependencies: "544KB"
info Disk size with unique dependencies: "1.21MB"
info Disk size with transitive dependencies: "1.25MB"
info Number of shared dependencies: 4
=> Found "@material-ui/core#jss@9.8.7"
info This module exists because "@material-ui#core" depends on it.
info Disk size without dependencies: "684KB"
info Disk size with unique dependencies: "824KB"
info Disk size with transitive dependencies: "884KB"
info Number of shared dependencies: 5

Your Environment 🌎

@material-ui/core@^4.0.0-alpha.1 and @material-ui/style@^4.0.0-alpha.1

eps1lon commented 5 years ago

Duplicate of #14297

VincentLanglet commented 5 years ago

@eps1lon I didn't found this issue (https://github.com/mui-org/material-ui/issues/14297)

But, the issue is closed and I don't consider a good fix, a solution like

"postinstall": "mkdir -p node_modules/@material-ui/core/node_modules/@types/jss && cp -r node_modules/@types/jss node_modules/@material-ui/core/node_modules/@types"

You said

Since we started working on v4 I think that working on this issue is not very productive. Once we release v4 we should use a single styling solution.

But now, it's happening on v4, so I think we should work on it.

Can't @material-ui/core and @material-ui/styles use the same version of jss ?

eps1lon commented 5 years ago

@VincentLanglet The current v4 release is not final. As I said that issue should be gone once v4 is stable.

I agree that this solution is not optimal but we can't do anything beyond that. I think I explained it in depth why this is happening and why we can't control this behavior.

Can't @material-ui/core and @material-ui/styles use the same version of jss ?

The core won't have a dependency on jss anymore. We can't upgrade that dependency to jss@10 in @material-ui/core@3 because that would be a breaking change.

oliviertassinari commented 5 years ago

I believe the root of the issue is with the conflict between JSS v9 and JSS v10. I'm killing JSS v9 in #14560. It should help. I want to release the change in v4.0.0-alpha.2.

VincentLanglet commented 5 years ago

@eps1lon Ok I didn't know @material-ui/core@4 wont have a dependency onjsssince the@material-ui/core@4-beta` still had one.

FYI, jss package is working on improving the typing https://github.com/cssinjs/jss/pull/973 It may help

Edit : @oliviertassinari Great ! I think it'll fix everything indeed

VincentLanglet commented 5 years ago

Using patch-package (or making a PR) is a solution with :

node_modules/@material-ui/core/styles/createGenerateClassName.d.ts

-import { GenerateClassName } from 'jss';
+import { GenerateId } from 'jss';

...

-export default function createGenerateClassName(options?: GenerateClassNameOptions): GenerateClassName;
+export default function createGenerateClassName(options?: GenerateClassNameOptions): GenerateId;

node_modules/@material-ui/core/styles/jssPreset.d.ts

-import { JSSOptions } from 'jss';
+import { JssOptions } from 'jss';

-export default function jssPreset(): JSSOptions;
+export default function jssPreset(): JssOptions;

node_modules/@material-ui/core/styles/withStyles.d.ts

 export interface WithStylesOptions<ClassKey extends string = string>
-  extends JSS.CreateStyleSheetOptions<ClassKey> {
+  extends JSS.StyleSheetFactoryOptions {
VincentLanglet commented 5 years ago

@oliviertassinari I saw you merged your PR. Good job !

There was still one thing to do in order to stop using the @types/jss library. It's here: https://github.com/mui-org/material-ui/pull/14852

oliviertassinari commented 5 years ago

@types/jss is gone, one less dependency :)

bouncydragon commented 5 years ago

@oliviertassinari the issue still exist, here is my package.json

"dependencies": { "@material-ui/core": "^3.9.2", "@material-ui/styles": "^3.0.0-alpha.10", "@types/react-loadable": "^5.5.0", "@types/react-router-dom": "^4.3.1", "prettier": "^1.16.4", "react": "^16.8.4", "react-dom": "^16.8.4", "react-loadable": "^5.5.0", "react-router-dom": "^5.0.0", "react-scripts-ts": "3.1.0" } "devDependencies": { "@types/jest": "^24.0.11", "@types/node": "^11.11.3", "@types/react": "^16.8.8", "@types/react-dom": "^16.8.2", "eslint": "5.3.0", "eslint-config-airbnb-base": "13.1.0", "eslint-plugin-import": "^2.14.0", "typescript": "^3.3.3333" }

eps1lon commented 5 years ago

"dependencies": { "@material-ui/core": "^3.9.2", "@material-ui/styles": "^3.0.0-alpha.10"

@bouncydragon The issue is not fixed for older versions. Try @material-ui/core@^4.0.0-alpha.4 and @material-ui/styles@^4.0.0-alpha.4

bouncydragon commented 5 years ago

@eps1lon will there be any issues while developing a project using material ui in alpha?

eps1lon commented 5 years ago

@eps1lon will there be any issues while developing a project using material ui in alpha?

I can't make that promise for any version. You were already using an alpha (@material-ui/styles) so it seems you're ok with handling some rough edges that we're in the process of polishing.

bouncydragon commented 5 years ago

@eps1lon will there be any timeframe for the stable release of the alpha version?

oliviertassinari commented 5 years ago

@bouncydragon I hope we can release it within a month.