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

createSvgIcon refactor in @material-ui 4.9.9 breaking builds #1619

Closed
Kauabunga opened this issue Apr 4, 2020 · 10 comments · Fixed by #1629
Closed

createSvgIcon refactor in @material-ui 4.9.9 breaking builds #1619

Kauabunga opened this issue Apr 4, 2020 · 10 comments · Fixed by #1629
Labels
bug 🐛 Something isn't working good first issue Great for newcomers
Milestone

Comments

@Kauabunga
Copy link

Tech Version
@material-ui/pickers v4.0.0-alpha.4
material-ui v4.9.9

Expected behavior

Use createSvgIcon from utils export

import { createSvgIcon } from '@material-ui/core/utils';

instead of

import createSvgIcon from '@material-ui/core/internal/svg-icons/createSvgIcon';

@ https://github.com/mui-org/material-ui-pickers/blob/574944acbb6e3918803914078674d9d815fc260c/lib/src/_shared/icons/ArrowDropDownIcon.tsx#L2

Actual behavior

Module not found: Can't resolve '@material-ui/core/internal/svg-icons/createSvgIcon' in '/.../node_modules/@material-ui/pickers'

Refactored utility createSvgIcon in latest material-ui version breaks pickers svgs. This has been removed from the internal path in the following commit

mui/material-ui@8ea2df8

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 5, 2020

I was about to open an issue to encourage the change of API, I hadn't realized that it would have broken the component, sorry.

I wish it didn't happen. We could have avoided the problem by releasing the pickers at the same time, which mui/material-ui#19706 would enable.
The alternative would have been to do it in two steps, keep the internals, add the new API, migrate pickers, remove internal. But this sounds like x2 the among of work.

@dmtrKovalenko
Copy link
Member

I am happy that at least v3.2.10 is not affected by this change.

@icflorescu
Copy link

Is there any way I can help to speed up the release of an alpha.5 release to include this fix?

@digitalkaoz
Copy link

i mean its an easy change, currently its all broken :(

@oliviertassinari oliviertassinari added the good first issue Great for newcomers label Apr 6, 2020
@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 6, 2020

@icflorescu Definitely, what do you think about this diff? Do you want to submit a pull request :)?

diff --git a/docs/package.json b/docs/package.json
index 6bde60d5..722ebeb3 100644
--- a/docs/package.json
+++ b/docs/package.json
@@ -21,7 +21,7 @@
     "@date-io/hijri": "^2.2.0",
     "@date-io/jalaali": "^2.0.0",
     "@mapbox/rehype-prism": "^0.4.0",
-    "@material-ui/core": "^4.9.7",
+    "@material-ui/core": "^4.9.9",
     "@material-ui/icons": "^4.9.1",
     "@material-ui/pickers": "^4.0.0-alpha.1",
     "@types/fuzzy-search": "^2.1.0",
diff --git a/lib/package.json b/lib/package.json
index e4477f69..6b8815b2 100644
--- a/lib/package.json
+++ b/lib/package.json
@@ -32,7 +32,7 @@
     "email": "[email protected]"
   },
   "peerDependencies": {
-    "@material-ui/core": "^4.0.0",
+    "@material-ui/core": "^4.9.9",
     "@types/react": "^16.8.6",
     "react": "^16.8.4",
     "react-dom": "^16.8.4"
@@ -77,7 +77,7 @@
     "@babel/preset-env": "^7.6.0",
     "@babel/preset-react": "^7.0.0",
     "@cypress/webpack-preprocessor": "^4.1.1",
-    "@material-ui/core": "^4.9.7",
+    "@material-ui/core": "^4.9.9",
     "@material-ui/icons": "^4.9.1",
     "@types/enzyme": "^3.10.5",
     "@types/enzyme-adapter-react-16": "^1.0.3",
diff --git a/lib/rollup.config.js b/lib/rollup.config.js
index 0628df6f..7977601a 100644
--- a/lib/rollup.config.js
+++ b/lib/rollup.config.js
@@ -26,7 +26,6 @@ const globals = {
   '@material-ui/core/Grid': 'material-ui.Grid',
   '@material-ui/core/IconButton': 'material-ui.IconButton',
   '@material-ui/core/InputAdornment': 'material-ui.InputAdornment',
-  '@material-ui/core/internal/svg-icons/createSvgIcon': 'material-ui.createSvgIcon',
   '@material-ui/core/Paper': 'material-ui.Paper',
   '@material-ui/core/Popover': 'material-ui.Popover',
   '@material-ui/core/styles': 'material-ui',
diff --git a/lib/src/_shared/icons/ArrowDropDownIcon.tsx b/lib/src/_shared/icons/ArrowDropDownIcon.tsx
index 1515a3f8..e5c0c589 100644
--- a/lib/src/_shared/icons/ArrowDropDownIcon.tsx
+++ b/lib/src/_shared/icons/ArrowDropDownIcon.tsx
@@ -1,4 +1,4 @@
 import React from 'react';
-import createSvgIcon from '@material-ui/core/internal/svg-icons/createSvgIcon';
+import { createSvgIcon } from '@material-ui/core/utils';

 export const ArrowDropDownIcon = createSvgIcon(<path d="M7 10l5 5 5-5z" />, 'ArrowDropDownIcon');
diff --git a/lib/typings.d.ts b/lib/typings.d.ts
index d58eafa3..e9750fd6 100644
--- a/lib/typings.d.ts
+++ b/lib/typings.d.ts
@@ -4,11 +4,3 @@ declare module '@date-io/type' {

   export type DateType = Moment | DateTime | Date;
 }
-
-declare module '@material-ui/core/internal/svg-icons/createSvgIcon' {
-  import * as React from 'react';
-  import { SvgIconProps } from '@material-ui/core';
-
-  declare const createSvgIcon: (path: React.ReactNode, name: string) => React.FC<SvgIconProps>;
-  export default createSvgIcon;
-}
diff --git a/yarn.lock b/yarn.lock
index c16e430f..32421057 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -1626,10 +1626,10 @@
     refractor "^2.3.0"
     unist-util-visit "^1.1.3"

-"@material-ui/core@^4.9.7":
-  version "4.9.7"
-  resolved "https://registry.yarnpkg.com/@material-ui/core/-/core-4.9.7.tgz#0c1caf123278770f34c5d8e9ecd9e1314f87a621"
-  integrity sha512-RTRibZgq572GHEskMAG4sP+bt3P3XyIkv3pOTR8grZAW2rSUd6JoGZLRM4S2HkuO7wS7cAU5SpU2s1EsmTgWog==
+"@material-ui/core@^4.9.9":
+  version "4.9.9"
+  resolved "https://registry.yarnpkg.com/@material-ui/core/-/core-4.9.9.tgz#902dc37eeb415dd3288feb2c92e0dc27d9caed48"
+  integrity sha512-Gp0UdJLxPEnkn7O0QpJ2/LOeIuT8nX9e6CjQFuLnOy10rUGjRsOZ2T170Y057xdUmw1VNE+0bvkkO6dOghxt4g==
   dependencies:
     "@babel/runtime" "^7.4.4"
     "@material-ui/styles" "^4.9.6"

It would need to be completed with all the icons.

@oliviertassinari oliviertassinari added this to the v4 milestone Apr 6, 2020
@kelly-tock

This comment has been minimized.

dmtrKovalenko added a commit that referenced this issue Apr 9, 2020
* Make value for picker be a generic type

* Spread new props from passing to the dom element

* Split makePickerWith state hoc

* Better type inference for new makePickerWithWrapper

* Refactor and opimize shared prop types inference for pickers

* Rename eome type names and files

* Integrate validation back to usePickerState

* Remove DateRangePicker code

* Remove DateRangePickerUsage from index module

* Fix small linter and ts errors

* Fix build erros

* Run prettier manually

* Display calendars for date range

* [WiP] Work on displaying 2 calendars one by one

* WATER ON MY MAC

* Proper display days with margins with range highlights

* Fix unexpected month switchings

* Implement range preview

* Optimize rerenders of Day and DateRangeDay, closes #1306

* Fix crash and missing rerender

* Optimize rerendering performace of <Day />

* Use popper for date range wrapper

* Proper displaying values in DateRangePIckerInput

* Implement ClickAwayListner for popper wrapper

* Fix slide animation in date range picker

* Reorganize test folders

* Reorganize integration test folder

* Add focus managment logic

* Implement autoscrolling for switching between range start/end

* Improve date range picker with disabling min/max dates

* Update date-io adapter version to 2.5.0

* Do not display range preview on highlighted days

* Better focus and blur handling for popper

* Fix range preview border style

* Fix type erros

* Fix fantom borders appearing

* Better styling for popper and possibility to replace transition

* Fix missing mui globals

* Fix werid memoization issue in Safari

* Implement mobile version of DateRangePicker

* Better displaying in mobile mode

* Make possible to switch view by clicking on month and year header

* Properly spread props to mobile keyboard input view

* Fix ts erros and enable api generation

* Fix example ts-checks

* Fix ts error in ServerRerquest.example.jsx

* Fix error if mouning in open state

* Add props test for DateRangePickers

* Export responsive date range picker by default

* Add missing displayName to the ToolbarButton

* Small enhancmenets

* Fix inclusivity of range for non-datefns livs

* Fix ts error

* Remove fake data from range example

* Use tsx in date range picker examples

* Add `startText` and `endText` props

* Update material-ui and fix createSvgIcon import, closes #1619

* Use new TextField `focused` prop for highlighting

* Close picker after range selected

* Update jss version

* Add more date-range-manager tests

* More examples and possibility to change wrapper mode for static wrapper

* Fix ts wrapper props inferring

* Update lib/src/views/Calendar/CalendarView.tsx

Co-Authored-By: Olivier Tassinari <[email protected]>

* Update docs/pages/demo/daterangepicker/CalendarsDateRangePicker.example.tsx

Co-Authored-By: Olivier Tassinari <[email protected]>

* Update createSvgIcon imports

* Fix ts inferrence for static wrapper props

* Add missing props spreading

* Fix more build errors

* Update lib/src/__tests__/setup.js

Co-Authored-By: Olivier Tassinari <[email protected]>

* Address review

* forwardRef for DateRangePicker

Co-authored-by: Olivier Tassinari <[email protected]>
@hadasmaimon
Copy link

@oliviertassinari
I got the same error
in "@material-ui/pickers": "4.0.0-alpha.7"
image

@oliviertassinari
Copy link
Member

oliviertassinari commented May 10, 2020

@hadasmaimon Check your peer dependency warning, resolve it.

@hadasmaimon
Copy link

yes you right!
has missing dependency: "luxon": "^1.21.3"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working good first issue Great for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants