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

feat(makeServerFetchye): adding run method to response #92

Merged

Conversation

jjallwood
Copy link
Contributor

We have been experimenting with using fetchye across our tenancy, and have run into an interesting case where we want to be able to use both the data, and the reload run methods as part of our module thunk methods. Currently we get both the data object and the run method into our components via the useFetchye hook, we then pass these into our redux thunks as parameters. However this method is clumsy and starts to get heavy when the example includes more than one source of data (we managed to get up to 3x in one case), or when thunks begin to chain calls.

This change is to export the run method, so both the two interfaces to Fetchye expose the same API to the client. This would allow us to re-use the underlying makeOneServerFetchye methods not only in our loadModuleData functions, but in redux thunks to load / get / reload data without getting stuck in parameter hell.

Since we have started this change the oneFetchye method was added to fetchye-one-app which compliments our change allowing engineers to just dispatch their fetchye requests in their thunk.

What we have today

const onPrimaryTravelerSaveThunk = ({
  contactInfo, // fetchye run and data are renamed and passed as a param to the thunk
  reloadContactInfo, 
}) => async (dispatch, getState) => {
  const state = getState();
  await dispatch(setFormSubmitted(true));

  if (hasFormChanged(contactInfo, state)) {
    dispatch(setIsSaving());
    try {
      await dispatch(postUpdatesThunk());
      await dispatch(setSaveSuccessful());
    } catch (error) {
      await dispatch(setErrorSaving());
    } finally {
      reloadContactInfo();
    }
  }
  dispatch(setFormSubmitted(false));
};

What we would like to be able to do

const onPrimaryTravelerSaveThunk = () => async (dispatch, getState) => {
  const state = getState();
  await dispatch(setFormSubmitted(true));

  const {
    data: contactInfo,
    run: reloadContactInfo,
  } = await dispatch(loadContactInfo());  // by accessing data directly from fetchye via a thunk. we can remove the parameters

  if (hasFormChanged(contactInfo, state)) {
    dispatch(setIsSaving());
    try {
      await dispatch(postUpdatesThunk());
      await dispatch(setSaveSuccessful());
    } catch (error) {
      await dispatch(setErrorSaving());
    } finally {
      reloadContactInfo();
    }
  }
  dispatch(setFormSubmitted(false));
};

Here is how we access the exported data, errors and run response within a thunk context via our loadContactInfo function

export const loadContactInfo = ({
  isCompanion,
  companionId,
}) => async (dispatch, getState) => {
  const { data: profileId } = await dispatch(loadProfileId);
  const { data: secToken } = await dispatch(loadSecToken);
  return dispatch(oneFetchye(
    getContactInfoUrl(profileId, isCompanion, companionId)(getState()),
    {
      headers: generateHeaders(secToken),
      credentials: 'include',
    }
  ));
};

@jjallwood jjallwood requested review from a team as code owners February 9, 2024 13:59
@CLAassistant
Copy link

CLAassistant commented Feb 9, 2024

CLA assistant check
All committers have signed the CLA.

Copy link

codesandbox-ci bot commented Feb 9, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit a5cb219:

Sandbox Source
quick-install Configuration
fetchye-provider-install Configuration
fetchye-redux-provider-install Configuration
nextjs-fetchye-ssr Configuration

});
expect(global.fetch).toHaveBeenNthCalledWith(2, 'http://example.com', {
headers: {
dynamicHeader: 'dynamic value 1',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@code-forger code-forger requested a review from a team February 14, 2024 09:52
@Matthew-Mallimo Matthew-Mallimo merged commit f6f9d94 into americanexpress:main Feb 14, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants