Links to component class about component-inspector

[lahmatiy]

Looks something changed in react and class resolving doesn't work (https://github.com/lahmatiy/component-inspector/blob/master/src/react/inspector/template-info/index.js#L73)
What is your React version?

[vslinko]

0.14.0-rc1

[lahmatiy]

Ok. I'll check it.
Also I see no code snippets in popup :(

Coming back to work on it later this week. Fixing issues as well.

[lahmatiy]

I checked on 0.14.0-rc1 – it's works as expected. No link to class it means no meta information was attached to class.
Looks like problem with instrumenting here. Could you provide example of class definition you use?

[vslinko]

I'm using es6 classes. Also I'm using a lot of component decorators. Some of my components have 4 or 5 higher order components on them attached via decorators.

import React, {PropTypes} from 'react';
import testable from 'frontend/utils/testable';

@testable()
export default class SignInForm extends React.Component {
  static propTypes = {
    // ...
  }

  render() {
    return (
      <form>
       {/* ... */}
      </form>
    );
  }
}

[lahmatiy]

Still works to me :-/
Could you provide instrumented version of your example?
What version of babel and babel-plugin-source-wrapper? Is babel-plugin-source-wrapper goes before any other plugins (except babel-plugin-react-display-name)?

Also you can find an universal version in master, as I promised. All react specifics could be found in https://github.com/lahmatiy/component-inspector/blob/master/src/api-adapter/react.js – maybe some mistake here (but I believe it's an instrumentation problem).

[vslinko]

Variant 1: Class found, render method found, problem in source code.

export default class Test extends React.Component {
  render() {
    return <div>qwe</div>
  }
}

Variant 2: Class and render method are found, but they are referenced to wrapped class. Problem in source code still exists.

function decorator(Component) {
  return class Wrapper extends React.Component {
    render() {
      return <Component />
    }
  }
}

@decorator
export default class Test extends React.Component {
  render() {
    return <div>qwe</div>
  }
}

Variant 3 (decorator from node_modules): class not found.

import {connect} from 'react-redux';

@connect()
export default class Test extends React.Component {
  render() {
    return <div>qwe</div>
  }
}

[vslinko]

So, class not found because inspector looking for a wrapped class, but that wrapped class doesn't instrumented.

[vslinko]

Why source code contains html? I don't know.

I'm using:

    "component-inspector": "^1.1.0",
    "express-open-in-editor": "^1.0.0",
    "babel-plugin-source-wrapper": "^1.1.1",

[lahmatiy]

What about babel version? I know resulting code for classes was changed some versions ago.

Why source code contains html? I don't know.

It tries get original source by request to server. Requested file is showing in popup header (/src/long/path/to/LocaleButtonsContainer.js on your screenshot). I suppose server response with html instead of original file.

I think we should find way to tell inspector to use bundle source (with its source map) if possible.

[lahmatiy]

Thank you for your helpful examples.
Variants 2 and 3 are requiring investigation and improvements. I didn't know decorators could replace class it applied to for new one. Looks like a hack still :)

[vslinko]

What about babel version?

    "babel": "^5.8.23",

I suppose server response with html instead of original file.

Yes, you're right, it works after I configured file serving. But I don't like this.

Looks like a hack still

It's common practice in React ecosystem. Those classes called Higher Order Components like higher order functions.

[lahmatiy]

As I realised general problem is caused using decorators (HOC actually).
Problem #3 is exists because connect creates new class in react-redux and I think this code isn't instrumented. I suppose only user code instrumenting now. Probably problem will gone when HOC will be processed as expected.

Yes, you're right, it works after I configured file serving. But I don't like this.

Initial solution was designed for basis.js, where no bundle in dev mode. And module subsytem has source of any executed module, as evaluate it on fly.
But I think we could improve solution here. Some ideas:

• scan document searching for any scripts, fetch those files and build modules map if possible. Ideally move it to Web Worker (or even to Service Worker in future). But additional requests to files are still required.
• middleware for dev-server (express or whatever), that serving source fragments by request. Maybe express-open-in-editor may be extended, or new middleware that would provide all necessary functionality.

Second one looks better.

[vslinko]

Probably problem will gone when HOC will be processed as expected.

Problem is gone, but there are two more issues:

1. Processing vendor code using Babel is too slow.
2. Class link is still referenced to wrapper component.

Second one looks better.

I agree.

[lahmatiy]

Problem is gone, but there are two more issues

It seems we have a misunderstanding. Problem with HOC is not solved yet (but just disscused with colleagues how to fix it). And when it will be fixed, it does not need to instrumenting libraries (in most cases).

I agree.

I'm going to implement new middleware for code fragment extracting. And one more that mix two middlewares (new one and express-open-in-editor) to simplify setup.

[vslinko]

Great news, I'll wait.

[lahmatiy]

Mostly done on decorators. It requires new version of babel plugin required (not published yet). Also new custom viewer for details was implemented. Believe you'll like it :)

Some edge cases are left to fix. And I'm going to release it (and babel plugin for instrumenting). New express middlewares are coming next.

It would be great if you able to try it on your code base until release.

[vslinko]

Installed babel plugin and inspector from master branches — no decorators support.

BTW your screenshot is great!

[vslinko]

Looks like you forgot rebuild inspector.

[lahmatiy]

You're right. Fixed.

[lahmatiy]

Just notice component name on your screenshot: <Connect(Connect(...(SignInForm)))>. Interesting how does it produced. I believe <SignInForm> should be shown. But probably with decorator support it will be ok.

[vslinko]

Basic decorator support is working:

Issues:

1. render and class is shifted by 1:
   
   First decorator is showing class of the component, second decorator is showing class of the first decorator.
2. It works with ES6 decorator syntax, but it doesn't work with classic decoration:

function decorator() {}
class Component {}
export default decorator(Component);

[lahmatiy]

Thank you for feedback.

1. It's simple bug, will fix asap.
2. This issue is more complicated. Since there is no explicit evidences decorator is used. For static analysis it's just a function call. So I have no idea for now how to link wrapped class with class produced by "decorator" and ignore irrelevant cases :-/

[vslinko]

Is it possible to separate those components to different levels?

<Connect(Testable(LocaleButtons))>
  <Testable(LocaleButtons)>
    <LocaleButtons>
    </LocaleButtons>
  </Testable(LocaleButtons)>
</Connect(Testable(LocaleButtons))>

[vslinko]

Like React Devtools do:

[lahmatiy]

render and class is shifted by 1:

Fixed 4db0260

[lahmatiy]

Is it possible to separate those components to different levels?

It's not clear to me. Do you really needs this?

<Connect(Testable(LocaleButtons))>
  <Testable(LocaleButtons)>
    ...
  </Testable(LocaleButtons)>
</Connect(Testable(LocaleButtons))>

I think it's noisy and not necessary. I want improve viewer for cases when wrapper adds extra makrup. But in simple cases virtual component is redundant.
Maybe I missed something. Could you provide more details?

[vslinko]

Do you really needs this?

This is definitely better then Connect(Testable(LocaleButtons)) = React.createClass.
I suggested this as an idea how to handle classic decorators.

[lahmatiy]

Hi!
1.2 is published as is.
Can you move your suggestions to new issue? So we could close this issue since this thread has too many q/a.

[vslinko]

done

[lahmatiy]

Thank you a lot!