Details of bundle changes

Generated by 🚫 dangerJS against ac953bb

I was going to report this, but @ankit beat me to it :)
@oliviertassinari Please review !

What do you think about this fix instead?

diff --git a/packages/material-ui/src/utils/scrollLeft.js b/packages/material-ui/src/utils/scrollLeft.js
index 2898642c4a..d336b0e4f6 100644
--- a/packages/material-ui/src/utils/scrollLeft.js
+++ b/packages/material-ui/src/utils/scrollLeft.js
@@ -18,7 +18,9 @@ export function detectScrollType() {
   }

   const dummy = document.createElement('div');
-  dummy.appendChild(document.createTextNode('ABCD'));
+  const container = document.createElement('div');
+  container.style.width = '100px';
+  container.style.height = '1px';
+  dummy.appendChild(container);
   dummy.dir = 'rtl';
   dummy.style.fontSize = '14px';
   dummy.style.width = '4px';

Is master the correct branch for a bugfix for 4.11? Will it be possible to publish a 4.11.1 release including this bugfix?

I would be leaning toward a target on the next branch. RTL has 2% of the download market share. Does it qualify for a critical bug fix? If you need it now, you can install: "@material-ui/core": "https://pkg.csb.dev/mui-org/material-ui/commit/f08eb4b7/@material-ui/core".

A better longterm fix would be to actually import https://github.com/alitaheri/normalize-scroll-left as a dependency, but I didn't go that route since I wasn't sure if we are deliberately trying to reduce the number of third-party dependencies.

I don't think that we need this dependency. The logic is getting normalized under a spec, browsers are progressively aligning:

diff --git a/packages/material-ui/src/utils/scrollLeft.js b/packages/material-ui/src/utils/scrollLeft.js
index 2898642c4a..926e2da037 100644
--- a/packages/material-ui/src/utils/scrollLeft.js
+++ b/packages/material-ui/src/utils/scrollLeft.js
@@ -6,11 +6,17 @@ let cachedType;
  *
-  * Types of scrollLeft, assiming scrollWidth=100 and direction is rtl.
+  * Types of scrollLeft, assuming scrollWidth=100 and direction is rtl.
  *
- * Browser        | Type          | <- Most Left | Most Right -> | Initial
--- a/packages/material-ui/src/utils/scrollLeft.js
+++ b/packages/material-ui/src/utils/scrollLeft.js
@@ -6,11 +6,17 @@ let cachedType;
  *
  * Types of scrollLeft, assiming scrollWidth=100 and direction is rtl.
  *
- * Browser        | Type          | <- Most Left | Most Right -> | Initial
- * -------------- | ------------- | ------------ | ------------- | -------
- * WebKit         | default       | 0            | 100           | 100
- * Firefox/Opera  | negative      | -100         | 0             | 0
- * IE/Edge        | reverse       | 100          | 0             | 0
+ * Type             | <- Most Left | Most Right -> | Initial
+ * ---------------- | ------------ | ------------- | -------
+ * default          | 0            | 100           | 100
+ * negative (spec*) | -100         | 0             | 0
+ * reverse          | 100          | 0             | 0
+ *
+ * Edge 85: default
+ * Safari 14: negative
+ * Chrome 85: negative
+ * Firefox 81: negative
+ * IE 11: reverse
+ *
+ * spec* https://drafts.csswg.org/cssom-view/#dom-window-scroll
  */

@oliviertassinari I think your fix is cleaner and less third-party dependencies is better
as for bug fix, well I think it qualifies, because 2% of market share is still a lot of people, plus even though you provided a package that includes the fix (thanks for that!), i'm guessing it would not be updated to include the rest of the changes that will happen until the next stable version is published
but it is your decision ultimately!!

because 2% of market share is still a lot of people

@nabc I would agree if we had an open issue about it with dozens of upvotes. It's not the case. Regarding v4, the next release will contain the deprecation messages we have done to reduce the gap between v4 and v5. But we can cherry-pick the diff to the master branch, it doesn't cost us much.

Thanks for the quick follow up and merging.

What do you think about this fix instead?

diff --git a/packages/material-ui/src/utils/scrollLeft.js b/packages/material-ui/src/utils/scrollLeft.js
index 2898642c4a..d336b0e4f6 100644
--- a/packages/material-ui/src/utils/scrollLeft.js
+++ b/packages/material-ui/src/utils/scrollLeft.js
@@ -18,7 +18,9 @@ export function detectScrollType() {
   }

   const dummy = document.createElement('div');
-  dummy.appendChild(document.createTextNode('ABCD'));
+  const container = document.createElement('div');
+  container.style.width = '100px';
+  container.style.height = '1px';
+  dummy.appendChild(container);
   dummy.dir = 'rtl';
   dummy.style.fontSize = '14px';
   dummy.style.width = '4px';

Thanks, does look much cleaner.

Is master the correct branch for a bugfix for 4.11? Will it be possible to publish a 4.11.1 release including this bugfix?

I would be leaning toward a target on the next branch. RTL has 2% of the download market share. Does it qualify for a critical bug fix? If you need it now, you can install: "@material-ui/core": "https://pkg.csb.dev/mui-org/material-ui/commit/f08eb4b7/@material-ui/core".

Thanks for the reference to using the commit hash for the dependency, very helpful.

A better longterm fix would be to actually import https://github.com/alitaheri/normalize-scroll-left as a dependency, but I didn't go that route since I wasn't sure if we are deliberately trying to reduce the number of third-party dependencies.

I don't think that we need this dependency. The logic is getting normalized under a spec, browsers are progressively aligning:

diff --git a/packages/material-ui/src/utils/scrollLeft.js b/packages/material-ui/src/utils/scrollLeft.js
index 2898642c4a..926e2da037 100644
--- a/packages/material-ui/src/utils/scrollLeft.js
+++ b/packages/material-ui/src/utils/scrollLeft.js
@@ -6,11 +6,17 @@ let cachedType;
  *
-  * Types of scrollLeft, assiming scrollWidth=100 and direction is rtl.
+  * Types of scrollLeft, assuming scrollWidth=100 and direction is rtl.
  *
- * Browser        | Type          | <- Most Left | Most Right -> | Initial
--- a/packages/material-ui/src/utils/scrollLeft.js
+++ b/packages/material-ui/src/utils/scrollLeft.js
@@ -6,11 +6,17 @@ let cachedType;
  *
  * Types of scrollLeft, assiming scrollWidth=100 and direction is rtl.
  *
- * Browser        | Type          | <- Most Left | Most Right -> | Initial
- * -------------- | ------------- | ------------ | ------------- | -------
- * WebKit         | default       | 0            | 100           | 100
- * Firefox/Opera  | negative      | -100         | 0             | 0
- * IE/Edge        | reverse       | 100          | 0             | 0
+ * Type             | <- Most Left | Most Right -> | Initial
+ * ---------------- | ------------ | ------------- | -------
+ * default          | 0            | 100           | 100
+ * negative (spec*) | -100         | 0             | 0
+ * reverse          | 100          | 0             | 0
+ *
+ * Edge 85: default
+ * Safari 14: negative
+ * Chrome 85: negative
+ * Firefox 81: negative
+ * IE 11: reverse
+ *
+ * spec* https://drafts.csswg.org/cssom-view/#dom-window-scroll
  */

Makes sense.