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

Use isomorphic fetch #4

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Use isomorphic fetch #4

wants to merge 2 commits into from

Conversation

lizard-boy
Copy link

Since fetch is stable in the current LTS version of Node, I don't see any good reason for using node-fetch or any other solution than the web standard.

@lizard-boy
Copy link
Author

@greptileai

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This pull request replaces the use of node-fetch with the native fetch API in the Notion SDK for JavaScript, aiming to simplify the codebase and reduce dependencies.

  • Increased minimum Node.js version requirement from 12 to 18 in package.json
  • Removed fetch and agent options from ClientOptions interface in src/Client.ts
  • Updated request method in src/Client.ts to use global fetch function
  • Removed src/fetch-types.ts file, eliminating custom fetch API type definitions
  • Updated errors.ts to use standard Headers type instead of custom SupportedResponse['headers']

4 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings

@@ -3,7 +3,7 @@
"version": "2.2.15",
"description": "A simple and easy to use client for the Notion API",
"engines": {
"node": ">=12"
"node": ">=18"
Copy link

Choose a reason for hiding this comment

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

logic: Increasing the minimum Node.js version to 18 may break compatibility for users on older versions

Copy link

Choose a reason for hiding this comment

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

style: Consider adding a comment explaining the transition to native fetch API for future maintainers

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.

2 participants