kjellmorten / hapi-json-api

Hapi plugin for enabling/enforcing JSON-API specification.
MIT License
44 stars 5 forks source link

Do not set JSON API header outside of the plugin #6

Closed gr2m closed 8 years ago

gr2m commented 8 years ago

see https://github.com/hoodiehq/hoodie-server-account/issues/8

Currently, when I load the plugin, static assets served using inert also get the content-type:application/vnd.api+json header.

Is there any way to limit that behavior to the plugin where I register @gar/hapi-json-api?

gr2m commented 8 years ago

I'm not very experienced with hapi yet, maybe I miss something, but an option similar to server.ext’s sandbox option would be great

gr2m commented 8 years ago

I've tried this

exports.register = function (plugin, options, done) {

    internals.meta = options.meta || {};

    plugin.ext('onPreHandler', internals.onPreHandler, {sandbox: 'plugin'});
    plugin.ext('onPreResponse', internals.onPreResponse, {sandbox: 'plugin'});

    return done();
};

But that limits it only to the hapi-json-api plugin, not the plugin registering it :/

wraithgar commented 8 years ago

Oof this looks like a tricky one. At first I thought label as the answer but that only is connection-level, not granular enough. I was at first cool on the idea presented in your pr #7 because of previous attempts at spec-compliance in relation to the proper accept headers. After a bit more thought however, since that parsing is all commented out and in the interest of getting things working now rather than later for someone actually using the library, I'm gonna merge it.

I know there's a way to do what you want, and eventually you'll wanna correct for that. But I think this fix is still the right move for now. I punted to @nlf on the hapi way to do it and he said:

so go here https://github.com/hapijs/hapi/blob/master/API.md#route-options

and scroll down and see between cors and files

the issue is those have to be set when you define the route, not when you register the plugin, but what i would do is just expose the methods from your plugin and document how to use it for the routes you want it

orrrrr, use a global extension point and have a filter for what routes it applies to

which is less work for the end user, but slightly uglier

i'd +1 what he's got, it's the simplest solution

So we can change it to that in the future if accept-headers ever become a pain point.

Finally, I'm in the 'add contributors early and often' camp so I'm gonna add you as a contributor here and on npm. This does not put any expectations on you going forward, it's simply an enabling of you (a person with interest in this project) to get things done when you need them.

You can merge and publish (patch version imo) at your leisure. Thank you for making this issue and PR.

gr2m commented 8 years ago

fixed via: https://github.com/wraithgar/hapi-json-api/commit/2705bb15b5fef689c6a872a8cc6d3ac8f2a3ec9f