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

How to consume Decorator Components in ES6 #925

Closed
veenedu opened this issue Mar 31, 2017 · 14 comments
Closed

How to consume Decorator Components in ES6 #925

veenedu opened this issue Mar 31, 2017 · 14 comments

Comments

@veenedu
Copy link

veenedu commented Mar 31, 2017

Hi,

I am using blueprintjs with facebook's create-react-app . I want to consume blueprint's HotkeysTarget. As per docs we have to add decorator to our Component class.

As of now create-react-app doesn't support decorators, read me

So how can I use HotkeysTarget and similar other component

@giladgray
Copy link
Contributor

so decorators are just functions with special syntax that invokes them for you, but they're very easy to unwind and call yourself. your "read me" link above links to two issues that provide acceptable workarounds for decorators in environments that don't support them.

this is the easiest way.

@giladgray
Copy link
Contributor

should be along the lines of export const MyHotkeysComponent = HotkeysTarget(MyComponent) 🎉

@veenedu
Copy link
Author

veenedu commented Mar 31, 2017

I tried both ways and none of them work.

//Method 1
const TestHotKeysComponent1 =  HotkeysTarget(TestComponent);

//TestHotKeysComponent1 is undefined 
console.log(TestHotKeysComponent1);


//Method 2
const TestHotKeysComponent2 =  HotkeysTarget(TestComponent);
//This throws an error. @HotkeysTarget-decorated class must implement `renderHotkeys`

Below is my TestComponent

import React, { Component } from 'react';
import {HotkeysTarget,Hotkey, Hotkeys,} from '@blueprintjs/core'

class TestComponent extends Component {
   
   //method copy pasted from blueprintJS Docs
    renderHotkeys(){
          return <Hotkeys>
            <Hotkey
                global={true}
                combo="shift + a"
                label="Be awesome all the time"
                onKeyDown={() => console.log("Awesome!")}
            />
            <Hotkey
                group="Fancy shortcuts"
                combo="shift + f"
                label="Be fancy only when focused"
                onKeyDown={() => console.log("So Fancy!")}
            />
        </Hotkeys>    
    }
    render() {        
        return (
            <div>
                Test Comp...
            </div>
        );
    }
}

const TestHotKeysComponent =  HotkeysTarget(TestComponent);

console.log(TestHotKeysComponent);

export default TestComponent;

@veenedu
Copy link
Author

veenedu commented Mar 31, 2017

@giladgray

In this file hotkeysTarget.tsx

If we add return this just before tslint:enable, it works fine.

Then we can use this component as const TestHotKeysComponent = HotkeysTarget(TestComponent);

I tried modifying the dist file directly in node modules and it worked. So I think it should work when we modify .tsx and re-run the build.

I modified this file.
node_modules/@blueprintjs/core/dist/components/hotkeys/hotkeysTarget.js

@giladgray
Copy link
Contributor

it looks like the decorator impl actually modifies the component prototype. this is a side effect, but it's always worked fine as a decorator.

class Component ... {}

HotkeysTarget(Component);
// done. Component is now augmented to support `renderHotkeys`.

feel free to submit a PR with the return statement if you want that usage.

veenedu added a commit to veenedu/blueprint that referenced this issue Mar 31, 2017
@veenedu
Copy link
Author

veenedu commented Apr 1, 2017

@giladgray
Hi, sorry to bug you again
There is no need to return this as I mentioned earlier

we can consume decorators in bluprintJS as below.

//define a component
class OrgComponent {}

//Just pass your component to decorator. 
HotkeysTarget(OrgComponent);

//Return original content (its now mutated by decorator)
export default OrgComponent

Ideally HOF should not mutate the original passed argument, but in this case they are.

@adidahiya
Copy link
Contributor

Ideally HOF should not mutate the original passed argument, but in this case they are.

huh, yeah, this is a bit strange and I didn't realize our hotkeys components did this. It's not how higher order components typically work, so we should consider refactoring HotkeysTarget and deprecating the existing API.

@veenedu
Copy link
Author

veenedu commented Apr 2, 2017

@adidahiya
ContextMenuTarget has same problem

@DrewDennison
Copy link
Contributor

Also ran into this issue and worked around it but would love to see the HOCs not mutate the component but return a new one for composing with Redux connect etc

@giladgray
Copy link
Contributor

closing as we've resolved how to use invoke decorator functions directly and filed #931 to track mutation issue.

@jennykortina
Copy link

jennykortina commented Aug 7, 2017

@giladgray when I try use ContextMenuTarget, instead of a decorator I wrap the component like so:

export default connect(null, dispatcher)(ContextMenuTarget(Component));

but i get this error:

Uncaught Error: You must pass a component to the function returned by connect. Instead received undefined at invariant

I'm importing ContextMenuTarget like this:

import { ContextMenuTarget, Menu, MenuItem } from '@blueprintjs/core';

Do you have any idea why this isn't working?

@adidahiya
Copy link
Contributor

@jennykortina because ContextMenuTarget doesn't return anything. For now you can work around this by doing this:

ContextMenuTarget(Component);
// at this point, Component's prototype has been modified by the decorator
export default connect(null, dispatcher)(Component);

The proper long-term fix is #931.

@jennykortina
Copy link

@adidahiya thank you!

@jordymeow
Copy link

Hello guys! I have the same issue as @veenedu (@HotkeysTarget-decorated class should implement renderHotkeys), and I am using exactly the same code as he mentioned. It also seems it has been fixed but as of version 3.6.1, I still have the issue. Is there something I should know?

Since googling about this issue brings to this page, it would be nice to have a clear and defined conclusion :) Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants