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

Break (some) components out into separately importable React components #26

Closed
rgbkrk opened this issue Jan 1, 2016 · 12 comments
Closed

Comments

@rgbkrk
Copy link
Member

rgbkrk commented Jan 1, 2016

I'd like to use and test these components in Electron and other environments. Not all apply since they rely on Atom's editor, quite a few do though. I'm happy to extract them with git history here within jupyter or over in nteract. Only suggesting nteract because I'm not sure if folks in Jupyter are sold on the idea of React, Immutable, and Redux. I am though, and fully invested in driving this forward.

What do you think @gnestor?

@rgbkrk
Copy link
Member Author

rgbkrk commented Jan 1, 2016

On a side note, I've also been working upstream on getting a zmq prebuilt available in JustinTulloss/zeromq.node#486.

@rgbkrk
Copy link
Member Author

rgbkrk commented Jan 1, 2016

Guess I should say it would be a collection of dumb pure components, without the dispatcher attached.

@gnestor
Copy link
Contributor

gnestor commented Jan 4, 2016

Sounds like a good idea. How can I help?

DisplayArea appears to be the only pure component. The others are coupled with Flux, Atom, Immutable, etc.

One way to decouple them from Flux is to replace instances of Dispatcher.dispatch({...}) with just this.dispatch({...}) and define dispatch in the component as an empty method that can be easily integrated with Flux by calling Dispatcher.dispatch(payload) or Redux using dispatch(payload). The same could be done to decouple from Atom (e.g. this.initSubscriptions() vs. this.subscriptions = new CompositeDisposable() in constructor and this.setGrammar() vs. this.textEditor = this.textEditorView.getModel()... in componentDidMount).

@rgbkrk
Copy link
Member Author

rgbkrk commented Jan 4, 2016

Over the last few days I ended up making some of these components, including a few dedicated to just the display data transformations:

After realizing how awkward transformime is with React for non-HTML mimetypes, I created react-transformime. The interface is a bit different, though here's how I've started using it:

import {
  richestMimetype,
  displayOrder, // Immutable.List of mimetypes
  transforms, // set of default mimetype -> React.Component in an Immutable.Map
} from 'transformime-react';

import Immutable from 'immutable';

const bundle = new Immutable.Map({'text/html': '<b>woo</b>'});
const mimetype = richestMimetype(bundle, displayOrder, transforms); 
const Transform = transforms.get(mimetype);

return <Transform data={bundle.get(mimetype)} />;

This is simpler if you use the default transforms and display order as well:

import { richestMimetype, transforms } from 'transformime-react';
import Immutable from 'immutable';

const bundle = new Immutable.Map({'text/html': '<b>woo</b>'});
const mimetype = richestMimetype(bundle); 
const Transform = transforms.get(mimetype);

return <Transform data={bundle.get(mimetype)} />;

Viewed as a whole, this is used within composition, a WIP Electron app of the notebook. I'm definitely interested in your opinions (and contributions) to the API for transformime-react.

@rgbkrk
Copy link
Member Author

rgbkrk commented Jan 4, 2016

The others are coupled with Flux, Atom, Immutable, etc.

Coupling with Immutable I think is a fine choice within this ecosystem of components. I've only worked with Redux and RxJS on React components, so I have no idea or opinion on how to handle abstracting around stores.

The editor part may be the harder one that you're outlining. As it stands, I've started using CodeMirror within composition even though I'd much rather be using the Atom API.

@rgbkrk
Copy link
Member Author

rgbkrk commented Jan 4, 2016

This made me think more about if we should provide a MimeBundle component, then I realized what the opinionated versions looked like.

All the representations

import React from 'react';
import Immutable from 'immutable';

import { transforms } from 'transformime-react';

export default class MimeBundle extends React.Component {
  static displayName = 'MimeBundle'

  static propTypes = {
    bundle: React.PropTypes.instanceOf(Immutable.Map).isRequired,
    transforms: React.PropTypes.instanceOf(Immutable.Map),
  }

  static defaultProps = {
    transforms,
  }

  render() {
    return (
      <div>
        {
          this.props.bundle.map((data, mimetype) => {
            const Transform = this.props.transforms.get(mimetype);
            return <Transform key={mimetype} data={data} />;
          }).toArray()
        }
      </div>
    );
  }
}

Only the richest

import React from 'react';
import Immutable from 'immutable';

import { richestMimetype, transforms, displayOrder } from 'transformime-react';

export default class RichestMime extends React.Component {
  static displayName = 'RichestMime'

  static propTypes = {
    bundle: React.PropTypes.instanceOf(Immutable.Map).isRequired,
    displayOrder: React.PropTypes.instanceOf(Immutable.List),
    transforms: React.PropTypes.instanceOf(Immutable.Map),
  }

  static defaultProps = {
    transforms,
    displayOrder,
  }

  render() {
    const mimetype = richestMimetype(
      this.props.bundle,
      this.props.displayOrder,
      this.props.transforms
    );

    const Transform = this.props.transforms.get(mimetype);
    const data = this.props.bundle.get(mimetype);
    return <Transform key={mimetype} data={data} />;
  }
}

Which, while we could include those, I realize this also means we could make neat ones that let you toggle through the mimetypes. Can't include that one since it would have interactive components.

@gnestor
Copy link
Contributor

gnestor commented Jan 6, 2016

I've also looked into whether it's possible to break out the <atom-text-editor> component and it hasn't been done yet, but it's good to know that it's possible!

Nice work on adapting transformime to React 👍

I'd like to run composition to see what you're talking about but I keep running into the same Babel error:

Uncaught Exception:
ReferenceError: [BABEL] /Users/grant/Sites/notebook/composition/app.js: Unknown option: /Users/grant/Sites/notebook/composition/package.json.presets
    at Logger.error (/Users/grant/Sites/notebook/composition/node_modules/electron-compilers/node_modules/babel-core/lib/transformation/file/logger.js:58:11)
    at OptionManager.mergeOptions (/Users/grant/Sites/notebook/composition/node_modules/electron-compilers/node_modules/babel-core/lib/transformation/file/options/option-manager.js:126:29)
    at OptionManager.addConfig (/Users/grant/Sites/notebook/composition/node_modules/electron-compilers/node_modules/babel-core/lib/transformation/file/options/option-manager.js:107:10)
    at OptionManager.findConfigs (/Users/grant/Sites/notebook/composition/node_modules/electron-compilers/node_modules/babel-core/lib/transformation/file/options/option-manager.js:171:32)
    at OptionManager.init (/Users/grant/Sites/notebook/composition/node_modules/electron-compilers/node_modules/babel-core/lib/transformation/file/options/option-manager.js:229:12)
    at File.initOptions (/Users/grant/Sites/notebook/composition/node_modules/electron-compilers/node_modules/babel-core/lib/transformation/file/index.js:147:75)
    at new File (/Users/grant/Sites/notebook/composition/node_modules/electron-compilers/node_modules/babel-core/lib/transformation/file/index.js:137:22)
    at Pipeline.transform (/Users/grant/Sites/notebook/composition/node_modules/electron-compilers/node_modules/babel-core/lib/transformation/pipeline.js:164:16)
    at BabelCompiler.compile (/Users/grant/Sites/notebook/composition/node_modules/electron-compilers/lib/js/babel.js:70:20)
    at BabelCompiler.loadFile (/Users/grant/Sites/notebook/composition/node_modules/electron-compilers/node_modules/electron-compile-cache/lib/compile-cache.js:239:19)

I understand that presets is a Babel 6 feature, but I definitely have Babel 6 installed locally and I've removed any global installations. Any ideas?

@rgbkrk
Copy link
Member Author

rgbkrk commented Jan 6, 2016

I wonder if that's an electron-compile issue. I've been using that to bootstrap and not have to run builds for sources.

Let me nuke my own dev environment and see what issues I pull up.

@rgbkrk
Copy link
Member Author

rgbkrk commented Jan 6, 2016

Totally reproducible after I nuked my node_modules and ran npm install again. Looks like some babel changes have made things interesting. I'll dig and fix this up. May end up pinning those more exactly.

@rgbkrk
Copy link
Member Author

rgbkrk commented Jan 6, 2016

Alright, try pulling the latest and see what happens. Sorry about that! I actually noticed some new babel plugin versions come out while I was setting up tests earlier.

@gnestor
Copy link
Contributor

gnestor commented Feb 16, 2016

@rgbkrk I'm gonna close this

@gnestor gnestor closed this as completed Feb 16, 2016
@rgbkrk
Copy link
Member Author

rgbkrk commented Feb 16, 2016

Sounds good, it's in a separate state now which is more about adopting other components.

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

No branches or pull requests

2 participants