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: can't load the form-data polyfill in browsers because fs doesn't exist #184

Merged
merged 1 commit into from
Oct 6, 2020

Conversation

erunion
Copy link
Contributor

@erunion erunion commented Oct 6, 2020

This fixes a bug in the recent multipart/form-data work I did in #173 where instead of detecting if the loaded FormData object from the form-data module is a polyfill, we were loading the polyfill directly from that library. Problem with this is that that file loads the fs module, which doesn't exist in the browser.

Instead, we're now just relying on the form-data module to give us the appropriate export (for browsers it'll load browser.js) it has given the environment HTTPSnippet is being run under.

Copy link
Contributor

@develohpanda develohpanda left a comment

Choose a reason for hiding this comment

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

👏

@develohpanda develohpanda merged commit 0fc7f4a into Kong:master Oct 6, 2020
@erunion erunion deleted the fix/fs-doesnt-exist branch October 6, 2020 23:07
@marcelltoth
Copy link

When can we expect a release including this fix?

@develohpanda
Copy link
Contributor

Hi @marcelltoth, likely late November or early December. Unfortunately the team is quite bogged down at the moment.

@marcelltoth
Copy link

Hi @marcelltoth, likely late November or early December. Unfortunately the team is quite bogged down at the moment.

That's sad to hear given that this is a complete blocker for any browser usage (it fails the build) and the fix is already merged.

But anyway, thanks @develohpanda for the quick reply, appreciated!

@develohpanda
Copy link
Contributor

develohpanda commented Nov 18, 2020

Sorry about that! I'll raise it tomorrow to see if we can push out a release sooner. This is a project my team inherited, and I haven't had a chance to automate the deployments yet. 😢

@erunion
Copy link
Contributor Author

erunion commented Nov 18, 2020

@marcelltoth If you're in a pinch for time you can use our fork that has the fix until it's published here. https://www.npmjs.com/package/@readme/httpsnippet

@develohpanda
Copy link
Contributor

Thanks @erunion! 1.24.0 has been published so you should be good to go, @marcelltoth!

@marcelltoth
Copy link

Thank you @develohpanda, you rock! 🎉

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