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

Allow relative urls for fastboot and ember-data #128

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 62 additions & 17 deletions addon/mixins/adapter-fetch.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
import Ember from 'ember';
import Mixin from '@ember/object/mixin';
import { assign, merge } from '@ember/polyfills'
import { assign, merge } from '@ember/polyfills';
import { computed, get } from '@ember/object';
import { getOwner } from '@ember/application';
import RSVP from 'rsvp';
import fetch from 'fetch';

const {
Logger: { warn }
} = Ember;

const httpRegex = /^https?:\/\//;
const protocolRelativeRegex = /^\/\//;
const RBRACKET = /\[\]$/;

/**
Expand Down Expand Up @@ -87,6 +91,7 @@ export function headersToObject(headers) {

return headersObject;
}

/**
* Helper function that translates the options passed to `jQuery.ajax` into a format that `fetch` expects.
* @param {Object} _options
Expand All @@ -97,7 +102,7 @@ export function mungOptionsForFetch(_options, adapter) {
// This allows this mixin to be backward compatible with Ember < 2.5.
const combineObjs = (assign || merge);
const options = combineObjs({
credentials: 'same-origin',
credentials: 'same-origin'
}, _options);

let adapterHeaders = adapter.get('headers');
Expand Down Expand Up @@ -134,6 +139,7 @@ export function mungOptionsForFetch(_options, adapter) {

return options;
}

/**
* Function that always attempts to parse the response as json, and if an error is thrown,
* returns an object with 'data' set to null if the response is
Expand Down Expand Up @@ -163,14 +169,28 @@ export function determineBodyPromise(response, requestData) {
}

export default Mixin.create({
/**
* @param {String} url
* @param {String} type
* @param {Object} _options
* @returns {Object}
* @override
*/
fastboot: computed(function() {
let owner = getOwner(this);
return owner && owner.lookup('service:fastboot');
}),

protocol: computed(function() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why use CPs with dependent keys? Can these be set in init?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link

Choose a reason for hiding this comment

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

For verifying if Fastboot is used isn't this the preferred way to do it in an addon—to avoid an error if the consumer doesn't have Fastboot installed?

One thing to note is that someone may use your addon in an app that doesn't have FastBoot installed. In that case, if your addon tries to inject the fastboot service, they'll get an exception saying the fastboot service cannot be found.

Instead, you can write a computed property that uses the low-level getOwner functionality to lookup the fastboot service directly.

https://ember-fastboot.com/docs/addon-author-guide#accessing-the-fastboot-service

Copy link
Member

Choose a reason for hiding this comment

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

@chrism Yea you're absolutely right, thansk for pointing out!

let protocol = get(this, 'fastboot.request.protocol');
// In Prember the protocol is the string 'undefined', so we default to HTTP
if (protocol === 'undefined:') {
protocol = 'http:';
}

return protocol;
}),

/**
* @param {String} url
* @param {String} type
* @param {Object} _options
* @returns {Object}
* @override
*/
ajaxOptions(url, type, options = {}) {
options.url = url;
options.type = type;
Expand All @@ -186,7 +206,7 @@ export default Mixin.create({
ajax(url, type, options) {
const requestData = {
url,
method: type,
method: type
};

const hash = this.ajaxOptions(url, type, options);
Expand Down Expand Up @@ -226,7 +246,7 @@ export default Mixin.create({
* @param {Object} options
*/
_fetchRequest(url, options) {
return fetch(url, options);
return fetch(this.buildServerURL(url), options);
},

/**
Expand All @@ -251,13 +271,38 @@ export default Mixin.create({
}
},

/**
* Determine fastboot compliant urls
* @param url
* @returns {*}
*/
buildServerURL(url) {
Copy link
Member

Choose a reason for hiding this comment

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

buildServerURL sounds Fastboot specific, maybe change the method name or move the non-fastboot branch to _fetchRequest? This function can also be unit tested.

if (!get(this, 'fastboot.isFastBoot')) {
return url;
}
let protocol = get(this, 'protocol');
let host = get(this, 'fastboot.request.host');

/**
* Allows for the error to be selected from either the
* response object, or the response data.
* @param {Object} response
* @param {Object} payload
*/
if (protocolRelativeRegex.test(url)) {
return `${protocol}${url}`;
} else if (!httpRegex.test(url)) {
try {
return `${protocol}//${host}${url}`;
Copy link
Member

Choose a reason for hiding this comment

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

When will this throw?

} catch (fbError) {
throw new Error(
'You are using Fetch Adapter with no host defined in your adapter. This will attempt to use the host of the FastBoot request, which is not configured for the current host of this request. Please set the hostWhitelist property for in your environment.js. FastBoot Error: ' +
fbError.message
);
}
}
},

/**
* Allows for the error to be selected from either the
* response object, or the response data.
* @param {Object} response
* @param {Object} payload
*/
parseFetchResponseForError(response, payload) {
return payload || response.statusTest;
},
Expand Down