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(fetch-requester): add @algolia/requester-fetch #1411

Merged
merged 2 commits into from Jul 6, 2022
Merged

feat(fetch-requester): add @algolia/requester-fetch #1411

merged 2 commits into from Jul 6, 2022

Conversation

ykzts
Copy link
Contributor

@ykzts ykzts commented Jun 24, 2022

Edge Computing such as Cloudflare Worker and Edge Functions (Vercel) do not support XHR or Node.js HTTP modules. add a requester that uses Fetch and Edge Computing. Algolia's JS client can be used with Edge Computing by adding a requester that uses Fetch.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 24, 2022

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 cbebf59:

Sandbox Source
javascript-client-app Configuration

@Haroenv
Copy link
Contributor

Haroenv commented Jun 24, 2022

First of all, thanks for this thorough pull request. It looks good on first glance, but I'll make sure to test it completely next week.

It makes sense for the future (even across major) to keep xhr as the default (as you've done here), as there's some other environments that don't have fetch (ie, react native)

Copy link
Contributor

@Haroenv Haroenv left a comment

Choose a reason for hiding this comment

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

this looks great and works as expected (sandbox that uses it: https://codesandbox.io/s/javascript-client-app-forked-iwbbje?file=/src/app.js). Some small nitpicks are all I have to add

shortcuts
shortcuts previously approved these changes Jul 6, 2022
Copy link
Member

@shortcuts shortcuts left a comment

Choose a reason for hiding this comment

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

Nothing to add, thanks a lot for the contribution!

package.json Outdated
@@ -50,6 +50,7 @@
"@wdio/static-server-service": "5.16.10",
"barrelsby": "2.2.0",
"bundlesize": "0.18.0",
"cross-fetch": "^3.1.5",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"cross-fetch": "^3.1.5",
"cross-fetch": "3.1.5",

We can let renovate do the things

yarn.lock Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@Haroenv Haroenv dismissed stale reviews from shortcuts and themself via cbebf59 July 6, 2022 14:02
@Haroenv Haroenv merged commit 7b62403 into algolia:master Jul 6, 2022
@ykzts ykzts deleted the feat/fetch-requester branch July 6, 2022 15:01
@my2ter
Copy link

my2ter commented Jan 19, 2023

Thanks for this!

Not sure if I'm missing something but unfortunately CloudFlare does not implement all parameters of the fetch api making it fails when I tried using it. I get a RetryError with the following:

The 'mode' field on 'RequestInitializerDict' is not implemented

If I comment out https://github.com/algolia/algoliasearch-client-javascript/blob/4.14.3/packages/requester-fetch/src/createFetchRequester.ts#L47 then it works.

Npm deps:

  • "@algolia/requester-fetch": "^4.14.3",
  • "algoliasearch": "^4.14.3"

Run locally: npx wrangler dev src/index.js
Code index.js:

import algoliasearch from 'algoliasearch';
import { createFetchRequester } from '@algolia/requester-fetch';

export default {
  async fetch(request) {
    return handleRequest(request)
  },
};

async function handleRequest(request) {
  const client = algoliasearch('APP_ID', 'SEARCH_KEY', {
    requester: createFetchRequester(),
  });

  const index = client.initIndex('index');
  let results = await index.search("a-search");

  return new Response(JSON.stringify(results), {
    headers: { 'content-type': 'application/json' },
  });
}

@shortcuts
Copy link
Member

shortcuts commented Jan 19, 2023

Hey @my2ter! we've change the order of the parameters in v5 (https://github.com/algolia/algoliasearch-client-javascript/blob/next/packages/requester-fetch/src/createFetchRequester.ts#L39-L49), which allows the user to override the value for the cors parameter, but it has not been reflected on v4.

If you wish to open the PR with the changes, please feel free! Otherwise I'll try to do it tomorrow

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

Successfully merging this pull request may close these issues.

None yet

4 participants