reflect-metadata
reflect-metadata copied to clipboard
Issue with IsConstructor when used with connect from react-redux
Line 543 should not check IsConstructor.
exporter("deleteMetadata", deleteMetadata);
function DecorateConstructor(decorators, target) {
for (var i = decorators.length - 1; i >= 0; --i) {
var decorator = decorators[i];
var decorated = decorator(target);
if (!IsUndefined(decorated) && !IsNull(decorated)) {
if (!IsConstructor(decorated))
throw new TypeError();
target = decorated;
}
}
return target;
}
As you can see on the image, decorated is a react class, hence typeof decorated is object and not function.
A React element is an object, but a React class component is definitely a function and a constructor. The issue here is that React.memo
doesn't return a function - you need to decorate the React component before wrapping it in memo
.
Can you provide an example of the class being decorated? DecorateConstructor
is expected to be called with a function
as its target (the "constructor"), so the IsConstructor
call looks to be correct.
Yes, I will prepare some test. What I have found so far, is that react-redux v5 indeed returns a function... but react-redux v7 (using the same code) returns that object. It might not be an issue with reflect-metadata at all but with react-redux. Thanks for your prompt support.
@rbuckton I think you may close this issue. reflect-metadata works as expected (sorry to bother you since the issue is from react-redux). If someone stumbles with this issue (related to react-redux) the problem is:
- Previous version of react-redux properly implements a connect class decorator (e.g react-redux 5.0.7) therefore, connect returns a wrapped function following class decorator specs.
- New version of react-redux (e.g react-redux 7.0.2) doesnt implements decorator logic and directly returns a react class (not a constructor function), and that is why IsConstructor fails in reflect-metadata.
I ended up solving this by using the connectAdvanced function (not the connect one) and properly implementing the decorator.
Another workaround is to stick with version react-redux 5.0.7
It seems like there are two options that control the output of connectAdvanced
that affect whether it can be used as a decorator:
-
options.pure
- Whentrue
, wraps the created constructor in a call toReact.memo
. Default:true
. -
options.forwardRef
- Whentrue
, wraps the previous result in a call toReact.forwardRef
. Default:false
.
Its possible using ~@connect({ pure: false, ...otherOptions })
~ might work for you. It depends on whether connect
is truly intended to be used as a decorator, as it seems that may no longer be the case.
Edit: sorry, try this instead:
import { createConnect } from 'react-redux/lib/connect/connect';
const connect = createConnect({ pure: false });
@connect(...)
class Foo {}
Awesome! Your tip works like a charm. Direct importing createConnect did not work for me, but if you pass { pure: false } as fourth param in connect, it works 👍
function decorator(args) {
// ... configure mapStateToProps, etc based on some args or pass them directly
return connect(
mapStateToProps,
mapDispatchToProps,
null,
{ pure: false })
)
@decorator(...)
class Foo {}
@rbuckton @aletc1
Setting pure: false
does work, but at the cost of not having your connected components being pure, which means you lose performance. See: https://react-redux.js.org/7.1/api/connect#pure-boolean
I think the best solution is not using connect
as decorator. Moreover if one uses TypeScript, connect
as decorator throws compiler errors and you can't even find a proper fix because it's discouraged in all issues I found. So, I'd simply use hoc or hooks pattern instead, although refactoring to the first will be much faster.
With hoc pattern, you can use compose
method from Redux, see: https://redux.js.org/api/compose
import { compose } from "redux"
export default compose(
connect,
withSubscribe,
// other Hocs...
)(MyComponent)
Closing this as External. Class decorators should always return either undefined
, or a replacement constructor. Returning a non-callable object
is not supported in either TypeScript's --experimentalDecorators
mode, nor in the Stage 3 ECMAScript Decorators proposal.