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

Fix bug: Error, Expected signal to be an instanceof AbortSignal #388

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

bernatfortet
Copy link

@bernatfortet bernatfortet commented Oct 13, 2023

  • update dependencies (which require a new build): node-fetch, abortcontroller-polyfill, typescript, es-lint
  • fix new errors/warnings caused by upgrading dependencies.
  • re-build airtable.browser

Reference issue: #386

package.json Outdated
@@ -1,6 +1,6 @@
{
"name": "airtable",
"version": "0.12.2",
"name": "bernatfortet.airtable",

Choose a reason for hiding this comment

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

Believe you will want to remove bernatfortet from this

Also is there any reason the version has been bumped to 0.12.5 instead of 0.12.3?

@vinayman
Copy link

vinayman commented Oct 17, 2023

@HongdaJiang-at @NabeelFarooqui-at - Any chance we can take a look at this being merged?

There is an issue in the description describing why we need this

@bernatfortet
Copy link
Author

@vinayman sorry for the late action. I've addressed the comments above.

@HongdaJiang-at @NabeelFarooqui-at could we see to merge this?

I see lots of related issues popping up.

@HongdaJiang-at
Copy link

@bernatfortet Thank you for the improvement and apology for the delayed response here! This change looks good to me!

@@ -26,7 +26,7 @@ type RecordCollectionCallback<TFields extends FieldSet> = (
error: CallbackError,
records?: Records<TFields>
) => void;
type DoneCallback = (error: CallbackError) => void;
type DoneCallback = (error: CallbackError, other?: any) => void;

Choose a reason for hiding this comment

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

Curious why do we need this change here?

Copy link
Author

Choose a reason for hiding this comment

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

This was throwing a ts error when used in a ts project.

@@ -1,6 +1,6 @@
{
"name": "airtable",
"version": "0.12.2",

Choose a reason for hiding this comment

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

Do you mind revert this change and rebuild airtable.browser.js)? since our release process will automatically bump this version number up :)

@bernatfortet
Copy link
Author

Hi @HongdaJiang-at I run the prepare command.
I also run test and I'm getting many errors, so unfortunately I'm not sure if this is ready to go out.

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.

3 participants