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

Jump to bottom

Feature/replace injector pattern with modules #85

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Feature/replace injector pattern with modules #85

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4290cb0
moved configManager to seperate file
mmnathan Nov 8, 2018
bff713e
moved gateway util to seperate file
mmnathan Nov 8, 2018
6a59389
replaced Injectables pattern with default exports
mmnathan Nov 8, 2018
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
17 changes: 4 additions & 13 deletions src/bootstrap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,17 @@ import 'modernizr';
import Vue from 'vue';
import VueI18nManager from 'vue-i18n-manager';
import { sync } from 'vuex-router-sync';

import './asset/style/screen.scss';

import './settings/settings';
import directive from './directive/directive';
import component from './component/component';
import getRouter from './router/router';
import getStore from './store/store';
import router from './router/router';
import store from './store/store';
import startUp from './control/startUp';
import getLocaleConfig from './config/localeConfig';
import setupInjects from './util/setupInjects';
import localeConfig from './config/localeConfig';
import localeLoader from './util/localeLoader';
import App from './App';
import filter from './filter/filter';
import App from './App';

// register filters globally
Object.keys(filter).forEach(key => Vue.filter(key, filter[key]));
Expand All @@ -26,17 +23,11 @@ Object.keys(directive).forEach(key => Vue.directive(key, directive[key]));
// register components globally
Object.keys(component).forEach(key => Vue.component(key, component[key]));

setupInjects();

if (window.webpackPublicPath) {
// eslint-disable-next-line
__webpack_public_path__ = window.webpackPublicPath;
}

const router = getRouter();
const store = getStore();
const localeConfig = getLocaleConfig();

if (localeConfig.localeEnabled) {
Vue.use(VueI18nManager, {
store,
Expand Down
47 changes: 20 additions & 27 deletions src/config/localeConfig.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,32 +1,25 @@
import { PropertyNames, VariableNames } from '../data/enum/configNames';
import { getValue } from '../util/injector';
import { CONFIG_MANAGER } from '../data/Injectables';
import configManager from '../util/configManager';

const getLocaleConfig = () => {
const configManager = getValue(CONFIG_MANAGER);
const languages = configManager.getProperty(PropertyNames.AVAILABLE_LOCALES).map(locale => {
if (typeof locale === 'string') {
return {
code: locale,
urlPrefix: locale,
translationKey: locale,
};
}
return locale;
});

const languages = configManager.getProperty(PropertyNames.AVAILABLE_LOCALES).map(locale => {
if (typeof locale === 'string') {
return {
code: locale,
urlPrefix: locale,
translationKey: locale,
};
}
return locale;
});

const config = {
persistent: false,
defaultCode: configManager.getProperty(PropertyNames.DEFAULT_LOCALE),
languages,
};

return {
localeEnabled: configManager.getVariable(VariableNames.LOCALE_ENABLED),
localeRoutingEnabled: configManager.getVariable(VariableNames.LOCALE_ROUTING_ENABLED),
config,
};
const config = {
languages,
persistent: false,
defaultCode: configManager.getProperty(PropertyNames.DEFAULT_LOCALE),
};

export default getLocaleConfig;
export default {
config,
localeEnabled: configManager.getVariable(VariableNames.LOCALE_ENABLED),
localeRoutingEnabled: configManager.getVariable(VariableNames.LOCALE_ROUTING_ENABLED),
};
14 changes: 4 additions & 10 deletions src/control/startUp.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,16 @@ import Vue from 'vue';
import axios from 'axios';
import DeviceStateTracker from 'seng-device-state-tracker';
import VueExposePlugin from '../util/VueExposePlugin';
import gateway from '../util/gateway';
import configManager from '../util/configManager';
import { URLNames, PropertyNames, VariableNames } from '../data/enum/configNames';
import { RouteNames } from '../router/routes';
import { createPath } from '../util/routeUtils';
import Params from '../data/enum/Params';
import { getValue } from '../util/injector';
import { CONFIG_MANAGER, GATEWAY } from '../data/Injectables';
import localeLoader from '../util/localeLoader';
import { mediaQueries, deviceState } from '../data/mediaQueries.json';

const initPlugins = () => {
const configManager = getValue(CONFIG_MANAGER);

const cleanMediaQueries = Object.keys(mediaQueries).reduce((result, key) => {
result[key] = mediaQueries[key].replace(/'/g, '');
return result;
Expand All @@ -22,7 +20,7 @@ const initPlugins = () => {
// expose objects to the Vue prototype for easy access in your vue templates and components
Vue.use(VueExposePlugin, {
$config: configManager,
$gateway: getValue(GATEWAY),
$gateway: gateway,
$http: axios,
$versionRoot: configManager.getVariable(VariableNames.VERSIONED_STATIC_ROOT),
$staticRoot: configManager.getVariable(VariableNames.STATIC_ROOT),
Expand Down Expand Up @@ -54,18 +52,14 @@ const waitForLocale = store =>
}
});

const startUp = store => {
export default store => {
// Initialise plugins
initPlugins();

const configManager = getValue(CONFIG_MANAGER);

// Add async methods to the Promise.all array
return Promise.all([
configManager.getVariable(VariableNames.LOCALE_ENABLED)
? waitForLocale(store)
: Promise.resolve(),
]);
};

export default startUp;
2 changes: 0 additions & 2 deletions src/data/Injectables.js
View file
Open in desktop

This file was deleted.

4 changes: 1 addition & 3 deletions src/data/enum/Params.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
const Params = {
export default {
Copy link
Owner

Choose a reason for hiding this comment

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

We had all enums like this in vue skeleton but after a comment of Narie we changed it to the const with the seperate export default. It had something to do with webstorm. I am not a big fan of a direct export default. I almost always use the const with the separate export default. So when you read the code you understand what is exported. In this case you have to look at the filename but doesn't always work because you can have multiple exports in one file.

Copy link

Choose a reason for hiding this comment

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

Yeah, webstorm has/d better autocomplete / autoimport if the const has the same name as the file/import name.

Copy link
Author

Choose a reason for hiding this comment

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

Fair point on the autocompletion, I did not consider this.

ID: 'id',
SLUG: 'slug',
};

export default Params;
90 changes: 38 additions & 52 deletions src/router/router.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,64 +1,50 @@
import VueRouter from 'vue-router';
import Vue from 'vue';
import VueRouter from 'vue-router';
import { routeParser } from 'vue-i18n-manager';
import localeConfig from '../config/localeConfig';
import configManager from '../util/configManager';
import { PropertyNames, VariableNames } from '../data/enum/configNames';
import getLocaleConfig from '../config/localeConfig';
import { CONFIG_MANAGER } from '../data/Injectables';
import { getValue } from '../util/injector';

import routes from './routes';

Vue.use(VueRouter);

let router = null;

const getRouter = () => {
Copy link

Choose a reason for hiding this comment

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

Are you aware that you are changing the behaviour of the running code with this change(s)?
Previously, the code in this function would only be executed when first called. Now it's executed when required (first thing). So will loose some control.

Copy link
Author

Choose a reason for hiding this comment

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

I see that the behaviour is changed but not how it affects the implementation, do you have an example of lost control?

Copy link

Choose a reason for hiding this comment

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

Let's say the router setup needs a value that is set in another file. You would like to control the order of how things are executed. Having to rely on changing the import order to (hopefully) change the order of execution is error-prone and unclear.
Explicitly calling functions to specify the order of execution gives you more control.

if (!router) {
const localeConfig = getLocaleConfig();
const configManager = getValue(CONFIG_MANAGER);

const processedRoutes =
localeConfig.localeEnabled && localeConfig.localeRoutingEnabled
? routeParser(routes, configManager.getProperty(PropertyNames.DEFAULT_LOCALE))
: routes.concat({
path: '*',
redirect: '/',
});

router = new VueRouter({
mode: 'history',
routes: processedRoutes,
base: configManager.getVariable(VariableNames.PUBLIC_PATH),
});

router.beforeEach((to, from, next) => {
const persistQueryParams = configManager.getProperty(PropertyNames.PERSIST_QUERY_PARAMS);

let redirect = false;
const { ...query } = to.query;

if (persistQueryParams && persistQueryParams.length > 0) {
persistQueryParams.forEach(queryParam => {
if (
typeof from.query[queryParam] !== 'undefined' &&
typeof query[queryParam] === 'undefined'
) {
query[queryParam] = from.query[queryParam];

redirect = true;
}
});
}

if (redirect) {
next({ path: to.path, query });
} else {
next();
const processedRoutes =
localeConfig.localeEnabled && localeConfig.localeRoutingEnabled
? routeParser(routes, configManager.getProperty(PropertyNames.DEFAULT_LOCALE))
: routes.concat({
path: '*',
redirect: '/',
});

const router = new VueRouter({
mode: 'history',
routes: processedRoutes,
base: configManager.getVariable(VariableNames.PUBLIC_PATH),
});

router.beforeEach((to, from, next) => {
const persistQueryParams = configManager.getProperty(PropertyNames.PERSIST_QUERY_PARAMS);

let redirect = false;
const { ...query } = to.query;

if (persistQueryParams && persistQueryParams.length > 0) {
persistQueryParams.forEach(queryParam => {
if (
typeof from.query[queryParam] !== 'undefined' &&
typeof query[queryParam] === 'undefined'
) {
query[queryParam] = from.query[queryParam];
redirect = true;
}
});
}

return router;
};
if (redirect) {
next({ path: to.path, query });
} else {
next();
}
});

export default getRouter;
export default router;
20 changes: 5 additions & 15 deletions src/store/store.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,10 @@
import Vuex from 'vuex';
import Vue from 'vue';
import Vuex from 'vuex';
import modules from './modules';

Vue.use(Vuex);

let store = null;

const getStore = () => {
if (!store) {
store = new Vuex.Store({
modules,
strict: process.env.NODE_ENV !== 'production',
});
}

return store;
};

export default getStore;
export default new Vuex.Store({
modules,
strict: process.env.NODE_ENV !== 'production',
});
7 changes: 7 additions & 0 deletions src/util/configManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import ConfigManager from 'seng-config';
import config from '../config/config';

const configManager = new ConfigManager();
configManager.init(config.config, config.environment);

export default configManager;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this file located in the util folder and not in the config folder?

Copy link
Author

@mmnathan mmnathan Nov 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Fair point. It used to be created and registered in util/setupInjects.js, I’ve separated the ‘injectables’ into separate files but felt relocating/refactoring should be another issue.

23 changes: 23 additions & 0 deletions src/util/gateway.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import axios from 'axios';
import { URLNames } from '../data/enum/configNames';
import configManager from './configManager';

const gateway = axios.create({
baseURL: configManager.getURL(URLNames.API),
headers: {
Accept: 'application/json',
},
responseType: 'json',
});

gateway.interceptors.response.use(
response =>
response && response.data && typeof response.data.data !== 'undefined'
? { ...response, ...response.data }
: response,
error => {
throw error;
},
);

export default gateway;
15 changes: 0 additions & 15 deletions src/util/injector.js
View file
Open in desktop

This file was deleted.

33 changes: 0 additions & 33 deletions src/util/setupInjects.js
View file
Open in desktop

This file was deleted.