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

Proposal: Access to app instance synchronously #424

Open
fahad19 opened this issue Jul 7, 2018 · 4 comments
Open

Proposal: Access to app instance synchronously #424

fahad19 opened this issue Jul 7, 2018 · 4 comments

Comments

@fahad19
Copy link
Member

fahad19 commented Jul 7, 2018

Currently

Components can only access the app instance via observe HoC, which also involves RxJS for streaming props:

import React from 'react';
import { observe } from 'frint-react';

function MyComponent() {
  return <p></p>;
}

export default observe(function (app, parentProps$) {
  return props$;
})(MyComponent);

Proposal

Not everyone needs to work with streaming props, and may only want to be able to access the app instance (which has the providers), and carry on with regular React lifecycle operations.

To achieve that, we can extend frint-react:

1: withApp function

import React from 'react';
import { withApp } from 'frint-react';

function MyComponent() {
  return <p></p>;
}

export withApp((app, props) => <MyComponent />);

2: WithApp component with render prop

This may not be necessary. Just withApp function alone should be enough.

import React from 'react';
import { WithApp } from 'frint-react';

export default function MyComponent() {
  return (
    <WithApp>
    {
      (app, props) => <p></p>
    }
    </WithApp>
  );
}
@fahad19
Copy link
Member Author

fahad19 commented Jul 7, 2018

@armand1m
Copy link
Member

armand1m commented Jul 7, 2018

I was speaking to @noisae and after some discussion, we thought that an API similar to redux connect would be nice since we could map what we do actually need from the App instead of injecting the whole App always.

This encourages developers using Frint to actually be more specific about what do they actually need from the App instead of relying on the whole app always.

Obviously the developer will still be able to do so, which there is also use cases that we don't have in mind, but implementing this can actually lead us to a path where we can encourage it and know more about what people actually use from their apps.

Perhaps something like:

/** ./MyComponent.js */

import React from 'react';

import FlightSearchStorePropType from '@prop-types/FlightSearchStorePropType';
import CheckoutStorePropType from '@prop-types/CheckoutStorePropType';

import { withApp } from 'frint-react';

const MyComponent = ({
  flightSearchStore,
  checkoutStore,
}) => (
  // render logic
);

MyComponent.propTypes = {
  flightSearchStore: FlightSearchStorePropType.isRequired,
  checkoutStore: CheckoutStorePropType.isRequired,
};

export const mapAppToProps = app => ({
  flightSearchStore: app.get('flightSearchStore'),
  checkoutStore: app.get('checkoutStore'),
});

export default withApp(mapAppToProps)(MyComponent);

This is also good since it makes it easier to test. You can export the mapAppToProps method and validate if it is calling the right resolvers for the expected providers.

/** ./MyComponent.test.js */

import { mapAppToProps } from './MyComponent';

const appMock = {
  get: jest.fn().mockImplementation(name => name),
};

describe('mapAppToProps', () => {
  it('should call correct resolvers', () => {
    mapAppToProps(appMock);
    expect(appMock.get).toBeCalledWith('flightSearchStore', 'checkoutStore');
  });

  it('should map to correct props' () => {
    const props = mapAppToProps(appMock);
    expect(props.flightSearchStore).toBe('flightSearchStore');
    expect(props.checkoutStore).toBe('checkoutStore');
  });
});

@fahad19
Copy link
Member Author

fahad19 commented Jul 7, 2018

@armand1m: Your suggestion makes sense, and adopting that kind of convention for Component modules can lead to more maintainable tests.

Actually, the current observe can already do this:

import React from 'react';
import { observe } from 'frint-react';

export const mapAppToProps = app => ({
  foo: app.get('foo'),
});

function MyComponent({ foo }) {
  return <p></p>;
};

export default observe(mapAppToProps)(MyComponent);

One thing that observe doesn't do is giving you access to received props synchronously (it gives you access to parent props as an Observable). That's a problem for people who don't want to get into RxJS. Something we could solve with a new withApp function may be without having to break observe HoC's API.

@armand1m
Copy link
Member

armand1m commented Jul 7, 2018

@fahad19: Agreed. The API would be pretty straight-forward in that sense,

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

2 participants