Skip to content

Koa multipart body #269

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Sep 1, 2017
Merged

Koa multipart body #269

merged 2 commits into from
Sep 1, 2017

Conversation

fabiob
Copy link
Contributor

@fabiob fabiob commented Aug 30, 2017

This PR fixes #156.

One of the new tests do fail when running under Gulp, but works fine when ran directly from mocha. I have no idea why.

@@ -111,6 +111,9 @@ export class KoaDriver extends BaseDriver implements Driver {
.forEach(param => {
defaultMiddlewares.push(multer(param.extraOptions).array(param.name));
});

// HACK: should be removed when koa-multer merges #15: https://github.com/koa-modules/multer/pull/15
Copy link
Member

Choose a reason for hiding this comment

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

No need to add the almost same comment twice, remove the comment here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@MichalLytek MichalLytek added type: fix Issues describing a broken feature. community labels Aug 30, 2017
@MichalLytek
Copy link
Contributor

Ping @NoNameProvided - merge it if you approve it now 😉

@NoNameProvided NoNameProvided merged commit 23a3af7 into typestack:master Sep 1, 2017
@NoNameProvided
Copy link
Member

Damn, thats why it bad to have failing tests, I assumed only the Cookie test fails on the latest stable but actually this PR brokes the added test in it. @fabiob did this works for you? It seems Koa fails the tests you wrote.

@MichalLytek
Copy link
Contributor

MichalLytek commented Sep 4, 2017

I can confirm that the test fails on newest node.js. Fortunately, only the newly added test fails so it won't break any existing feature 😆

Because it fails also in express and not on older node versions, it looks like the second weird test bug on newest node.

Bu the reason why both drivers return 400 Bad Request when using both decorators is that the default decorator option is required: true so there is an error on validation, dunno why 😩

@MichalLytek
Copy link
Contributor

@NoNameProvided
Looks like the multer that is parsing body, is placing files in req.body object that doesn't prototypal inherit from Object, so the validator throw empty errors.

I've fixed it in 5c7face, so now you can sleep safe & sound 😉

@fabiob fabiob deleted the koa-multipart-body branch September 4, 2017 14:08
@NoNameProvided
Copy link
Member

Thanks.

@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 12, 2020
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
type: fix Issues describing a broken feature.
Development

Successfully merging this pull request may close these issues.

How to get multipart/form-data text field?
3 participants