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.

Sign up for GitHub

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

feat: Remove deprecated return value on .js and .css methods #156

Merged
merged 1 commit into from
Jul 21, 2020
Merged

feat: Remove deprecated return value on .js and .css methods #156

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
63 changes: 9 additions & 54 deletions lib/layout.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,32 +23,11 @@ const Proxy = require('@podium/proxy');
const merge = require('lodash.merge');
const pkg = require('../package.json');

const _compabillity = Symbol('_compabillity');
const _pathname = Symbol('_pathname');
const _sanitize = Symbol('_sanitize');
const _addCssAsset = Symbol('_addCssAsset');
const _addJsAsset = Symbol('_addJsAsset');

function deprecateJsReturn() {
if (!deprecateJsReturn.warned) {
deprecateJsReturn.warned = true;
process.emitWarning(
'Return value from method js() is now deprecated and will be removed in a future version. Please do not rely on this value.',
'DeprecationWarning',
);
}
}

function deprecateCssReturn() {
if (!deprecateCssReturn.warned) {
deprecateCssReturn.warned = true;
process.emitWarning(
'Return value from method css() is now deprecated and will be removed in a future version. Please do not rely on this value.',
'DeprecationWarning',
);
}
}

const PodiumLayout = class PodiumLayout {
/* istanbul ignore next */
constructor({
Expand Down Expand Up @@ -185,18 +164,10 @@ const PodiumLayout = class PodiumLayout {
}

[_addCssAsset](options = {}) {
if (!options.value) {
const v = this[_compabillity](this.cssRoute);
return this[_sanitize](v, options.prefix);
}

const clonedOptions = JSON.parse(JSON.stringify(options));
const args = { ...clonedOptions, pathname: this._pathname };
this.cssRoute.push(new AssetCss(args));

// deprecate
deprecateCssReturn();
return this[_sanitize](args.value, args.prefix);
const clonedOptions = JSON.parse(JSON.stringify(options));
clonedOptions.value = this[_sanitize](clonedOptions.value, clonedOptions.prefix)
const args = { prefix: true, ...clonedOptions, pathname: this._pathname };
this.cssRoute.push(new AssetCss(args));
}

css(options = {}) {
Expand All @@ -206,17 +177,14 @@ const PodiumLayout = class PodiumLayout {
}
return;
}
return this[_addCssAsset](options);
this[_addCssAsset](options);
}

[_addJsAsset](options = {}) {
if (!options.value) {
const v = this[_compabillity](this.jsRoute);
return this[_sanitize](v, options.prefix);
}

const clonedOptions = JSON.parse(JSON.stringify(options));
const args = { ...clonedOptions, pathname: this._pathname };
clonedOptions.value = this[_sanitize](clonedOptions.value, clonedOptions.prefix)

const args = { prefix: true, ...clonedOptions, pathname: this._pathname };

// Convert data attribute object structure to array of key value objects
if (typeof args.data === 'object' && args.data !== null) {
Expand All @@ -231,10 +199,6 @@ const PodiumLayout = class PodiumLayout {
}

this.jsRoute.push(new AssetJs(args));

// deprecate
deprecateJsReturn();
return this[_sanitize](args.value, args.prefix);
}

js(options = {}) {
Expand All @@ -244,7 +208,7 @@ const PodiumLayout = class PodiumLayout {
}
return;
}
return this[_addJsAsset](options);
this[_addJsAsset](options);
}

view(fn = null) {
Expand Down Expand Up @@ -303,15 +267,6 @@ const PodiumLayout = class PodiumLayout {
}
return uri;
}

// This is here only to cater for compabillity between version 3 and 4
// Can be removed when deprecation of the .assets terminated
[_compabillity](arr) {
const result = arr.map(obj => {
return obj.value;
});
return result.length === 0 ? '' : result[0];
}
};

module.exports = PodiumLayout;
10 changes: 5 additions & 5 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@
},
"dependencies": {
"@metrics/client": "2.5.0",
"@podium/client": "4.4.2",
"@podium/context": "4.1.6",
"@podium/proxy": "4.2.1",
"@podium/schemas": "4.0.2",
"@podium/utils": "4.3.0",
"@podium/client": "5.0.0-next.4",
"@podium/context": "5.0.0-next.3",
"@podium/proxy": "5.0.0-next.2",
"@podium/schemas": "5.0.0-next.1",
"@podium/utils": "5.0.0-next.2",
"abslog": "2.4.0",
"lodash.merge": "4.6.2",
"objobj": "1.0.0"
Expand Down
121 changes: 24 additions & 97 deletions tests/layout.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ test('Layout() - metrics properly decorated', t => {
});

app.get('/', async (req, res) => {
const response = await podletClient.fetch(res.locals.podium.context);
const response = await podletClient.fetch(res.locals.podium);
res.send(response.content);
});

Expand Down Expand Up @@ -219,58 +219,21 @@ test('Layout() - metrics properly decorated', t => {
// .css()
// #############################################

test('.css() - call method with no arguments - should return default value', t => {
test('.css() - call method with no arguments - should throw', (t) => {
const layout = new Layout(DEFAULT_OPTIONS);
const result = layout.css();
t.equal(result, '');
t.end();
});

test('.css() - set legal value on "value" argument - should return set value', t => {
const layout = new Layout(DEFAULT_OPTIONS);
const result = layout.css({ value: '/foo/bar' });
t.equal(result, '/foo/bar');
t.end();
});

test('.css() - set "prefix" argument to "true" - should prefix value returned by method', t => {
const options = { ...DEFAULT_OPTIONS, pathname: '/xyz' };
const layout = new Layout(options);
const result = layout.css({ value: '/foo/bar', prefix: true });
t.equal(result, '/xyz/foo/bar');
t.end();
});

test('.css() - set legal absolute value on "value" argument - should set "css" to set value', t => {
const layout = new Layout(DEFAULT_OPTIONS);
const result = layout.css({ value: 'http://somewhere.remote.com' });
t.equal(result, 'http://somewhere.remote.com');
t.end();
});

test('.css() - set illegal value on "value" argument - should throw', t => {
const layout = new Layout(DEFAULT_OPTIONS);

t.throws(() => {
layout.css({ value: '/foo / bar' });
}, 'Value for argument variable "value", "/foo / bar", is not valid');
t.end();
});

test('.css() - call method with "value" argument, then call it a second time with no argument - should return first set value on second call', t => {
const layout = new Layout(DEFAULT_OPTIONS);
layout.css({ value: '/foo/bar' });
const result = layout.css();
t.equal(result, '/foo/bar');
t.end();
layout.css();
}, 'Value for argument variable "value", "undefined", is not valid');
t.end()
});

test('.css() - call method twice with a value for "value" argument - should set both values', t => {
test('.css() - set legal absolute value on "value" argument - should set "css" to set value', t => {
const layout = new Layout(DEFAULT_OPTIONS);
layout.css({ value: '/foo/bar' });
layout.css({ value: '/bar/foo' });
const result = layout.css();
t.equal(result, '/foo/bar');
layout.css({ value: 'http://somewhere.remote.com' });
const result = JSON.parse(JSON.stringify(layout.cssRoute));
t.same(result, [
{ rel: 'stylesheet', type: 'text/css', value: 'http://somewhere.remote.com' },
]);
t.end();
});

Expand Down Expand Up @@ -327,6 +290,14 @@ test('.css() - passing an instance of AssetsCss - should return set value', t =>
// .js()
// #############################################

test('.js() - call method with no arguments - should throw', (t) => {
const layout = new Layout(DEFAULT_OPTIONS);
t.throws(() => {
layout.js();
}, 'Value for argument variable "value", "undefined", is not valid');
t.end()
});

test('.js() - passing an instance of AssetsJs - should return set value', t => {
const layout = new Layout(DEFAULT_OPTIONS);
layout.js(new AssetJs({ value: '/foo/bar', type: 'module' }));
Expand All @@ -335,57 +306,13 @@ test('.js() - passing an instance of AssetsJs - should return set value', t => {
t.end();
});

test('.js() - call method with no arguments - should return default value', t => {
const layout = new Layout(DEFAULT_OPTIONS);
const result = layout.js();
t.equal(result, '');
t.end();
});

test('.js() - set legal value on "value" argument - should return set value', t => {
const layout = new Layout(DEFAULT_OPTIONS);
const result = layout.js({ value: '/foo/bar' });
t.equal(result, '/foo/bar');
t.end();
});

test('.js() - set "prefix" argument to "true" - should prefix value returned by method', t => {
const options = { ...DEFAULT_OPTIONS, pathname: '/xyz' };
const layout = new Layout(options);
const result = layout.js({ value: '/foo/bar', prefix: true });
t.equal(result, '/xyz/foo/bar');
t.end();
});

test('.js() - set legal absolute value on "value" argument - should set "js" to set value', t => {
const layout = new Layout(DEFAULT_OPTIONS);
const result = layout.js({ value: 'http://somewhere.remote.com' });
t.equal(result, 'http://somewhere.remote.com');
t.end();
});

test('.js() - set illegal value on "value" argument - should throw', t => {
const layout = new Layout(DEFAULT_OPTIONS);
t.throws(() => {
layout.js({ value: '/foo / bar' });
}, 'Value for argument variable "value", "/foo / bar", is not valid');
t.end();
});

test('.js() - call method with "value" argument, then call it a second time with no argument - should return first set value on second call', t => {
const layout = new Layout(DEFAULT_OPTIONS);
layout.js({ value: '/foo/bar' });
const result = layout.js();
t.equals(result, '/foo/bar');
t.end();
});

test('.js() - call method twice with a value for "value" argument - should set both values', t => {
const layout = new Layout(DEFAULT_OPTIONS);
layout.js({ value: '/foo/bar' });
layout.js({ value: '/bar/foo' });
const result = layout.js();
t.equals(result, '/foo/bar');
layout.js({ value: 'http://somewhere.remote.com' });
const result = JSON.parse(JSON.stringify(layout.jsRoute));
t.same(result, [
{ type: 'default', value: 'http://somewhere.remote.com' },
]);
t.end();
});

Expand Down