Skip to content

Commit

Permalink
FIX code review issues
Browse files Browse the repository at this point in the history
  • Loading branch information
ahmetkuslular committed Mar 1, 2022
1 parent f6a2827 commit 6482ec9
Show file tree
Hide file tree
Showing 14 changed files with 74 additions and 104 deletions.
1 change: 1 addition & 0 deletions config/emptyModule.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = {};
8 changes: 6 additions & 2 deletions config/string.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,16 @@ function replaceString() {
},
{
search: '__V_REQUEST_CONFIGS__',
replace: normalizeUrl(voltranConfig.routing.requestConfigs),
replace: normalizeUrl(
voltranConfig.routing.requestConfigs || path.resolve(__dirname, './emptyModule.js')
),
flags: 'g'
},
{
search: '__V_PREVIEW_PAGES__',
replace: normalizeUrl(voltranConfig.routing.previewPages),
replace: normalizeUrl(
voltranConfig.routing.previewPages || path.resolve(__dirname, './emptyModule.js')
),
flags: 'g'
},
{
Expand Down
7 changes: 5 additions & 2 deletions jest.config.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// Jest configuration
// https://facebook.github.io/jest/docs/en/configuration.html
module.exports = {
verbose: true,
automock: false,
Expand All @@ -11,6 +9,11 @@ module.exports = {
'!src/public/**',
'!src/tools/**'
],
env: {
production: {
plugins: ['transform-es2015-modules-commonjs']
}
},
coverageDirectory: '<rootDir>/coverage',
globals: {
window: true,
Expand Down
12 changes: 8 additions & 4 deletions lib/os.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
const path = require('path');

function normalizeUrl(url) {
const urlArray = url.split(path.sep);

return urlArray.join('/');
if (url) {
const urlArray = url?.split(path.sep);

return urlArray.join('/');
}

return '';
}

module.exports = normalizeUrl;
module.exports = normalizeUrl;
11 changes: 6 additions & 5 deletions src/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import xss from 'xss';

import { matchUrlInRouteConfigs } from './universal/core/route/routeUtils';
import Preview from './universal/components/Preview';
import { HTTP_STATUS_CODES } from './universal/utils/constants';
import { BLACKLIST_OUTPUT, HTTP_STATUS_CODES } from './universal/utils/constants';
import metrics from './metrics';
import {
renderComponent,
Expand All @@ -13,6 +13,7 @@ import {
} from './universal/service/RenderService';
import Component from './universal/model/Component';
import logger from './universal/utils/logger';
import omit from 'lodash/omit';

const appConfig = require('__APP_CONFIG__');

Expand Down Expand Up @@ -60,9 +61,9 @@ const render = async (req, res) => {
scripts,
activeComponent,
componentName,
seoState,
isPreviewQuery,
responseOptions
responseOptions,
...responseData
} = renderResponse;

const statusCode = responseOptions?.isPartialContent
Expand All @@ -71,8 +72,8 @@ const render = async (req, res) => {

if (!isPreview(context.query)) {
const html = renderLinksAndScripts(output, '', '');

res.status(statusCode).json({ html, scripts, style: links, activeComponent, seoState });
const otherParams = omit(responseData, BLACKLIST_OUTPUT);
res.status(statusCode).json({ html, scripts, style: links, activeComponent, ...otherParams });

metrics.fragmentRequestDurationMicroseconds
.labels(componentName, isWithoutHTML(context.query) ? '1' : '0')
Expand Down
2 changes: 1 addition & 1 deletion src/renderMultiple.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ async function getResponses(renderers) {

async function getPreview(responses, requestCount, req) {
const layoutName = getPreviewLayout(req.query);
const { layouts } = previewPages.default;
const { layouts = {} } = previewPages?.default || {};
let PreviewFile = Preview;

if (layouts[layoutName]) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { isExitCondition } from './RequestDispatcher.utils';

const requestConfigs = require('__V_REQUEST_CONFIGS__');

const { effects = [] } = requestConfigs.default;
const { effects = [] } = requestConfigs?.default || {};
const requestPrefix = 'RequestDispatcher.';
const responsePrefix = 'RequestDispatcher.Response.';

Expand Down
10 changes: 2 additions & 8 deletions src/universal/model/Renderer.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,7 @@
import omit from 'lodash/omit';
import { isPreview, renderComponent, renderLinksAndScripts } from '../service/RenderService';
import { BLACKLIST_OUTPUT } from '../utils/constants';

const blacklistOutput = [
'componentName',
'fullWidth',
'isMobileComponent',
'isPreviewQuery',
'responseOptions'
];
export default class Renderer {
constructor(component, context) {
this.component = component;
Expand Down Expand Up @@ -47,7 +41,7 @@ export default class Renderer {
return new Promise(resolve => {
renderComponent(this.component, this.context, this.initialState).then(response => {
const { output, links, fullHtml, ...rest } = response;
const otherParams = omit(rest, blacklistOutput);
const otherParams = omit(rest, BLACKLIST_OUTPUT);
const html = renderLinksAndScripts(output, '', '');

resolve({
Expand Down
2 changes: 1 addition & 1 deletion src/universal/partials/Welcome/PartialList.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const Welcome = () => {
<Link href={item.previewUrl ? item.previewUrl : `${item.url}?preview`} target="_blank">
<Name>{item.name}</Name>
<Url>{item.url}</Url>
<Footer>
<Footer status={item.status}>
<Label status={item.status}>
{item.status} <Dot status={item.status} />
</Label>
Expand Down
17 changes: 10 additions & 7 deletions src/universal/partials/Welcome/partials.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,18 @@ import components from '../../core/route/components';
const previewPages = require('__V_PREVIEW_PAGES__');

const partials = [];

const BLACKLIST = ['REQUEST_DISPATCHER'];
Object.keys(components).forEach(path => {
const info = components[path];
partials.push({
name: info.fragmentName,
url: path,
status: info.status
});
if (!BLACKLIST.includes(info.name)) {
partials.push({
name: info.fragmentName,
url: path,
status: info.status
});
}
});
partials.push(...previewPages.default.pages);
const pages = previewPages?.default?.pages || [];
partials.push(...pages);

export default partials;
11 changes: 5 additions & 6 deletions src/universal/partials/Welcome/styled.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,10 @@ export const ListItem = styled.li`
display: inline-block;
vertical-align: top;
height: 120px;
width: 320px;
width: 240px;
margin: 10px;
cursor: pointer;
border-radius: 20px;
border: 1px solid ${({ status }) => (status && STATUS_COLOR[status]) || '#8dc63f'};
border-radius: 10px;
position: relative;
background-color: #fff;
Expand All @@ -38,14 +37,14 @@ export const ListItem = styled.li`
:after {
content: '';
border-radius: 20px;
border-radius: 10px;
position: absolute;
z-index: -1;
top: 0;
left: 0;
width: 100%;
height: 100%;
box-shadow: 0 5px 15px rgba(0, 0, 0, 0.3);
box-shadow: 0 5px 15px rgba(0, 0, 0, 0.1);
opacity: 0;
-webkit-transition: all 0.6s cubic-bezier(0.165, 0.84, 0.44, 1);
transition: all 0.6s cubic-bezier(0.165, 0.84, 0.44, 1);
Expand Down Expand Up @@ -104,7 +103,7 @@ export const Footer = styled.span`
right: 0;
width: 100%;
padding: 20px;
border-top: 1px solid #eee;
border-top: 1px solid ${({ status }) => (status && STATUS_COLOR[status]) || '#eeeeee'}50;
font-size: 13px;
`;

Expand Down
11 changes: 7 additions & 4 deletions src/universal/service/RenderService.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,15 @@ const isWithoutHTML = query => {
};

const isPreview = query => {
return query.preview === '';
if (query.preview || query.preview === '') {
return true;
}
return false;
};

const getPreviewLayout = query => {
if (query?.previewLayout) {
return query?.previewLayout;
if (isPreview(query)) {
return query?.preview;
}

return '';
Expand All @@ -70,7 +73,7 @@ const isWithoutState = query => {
};

const isRequestDispatcher = query => {
return query.requestDispathcer === '' || query.requestDispathcer !== 'false';
return query.requestDispatcher === '' || query.requestDispatcher !== 'false';
};

const renderComponent = async (component, context, predefinedInitialState = null) => {
Expand Down
73 changes: 11 additions & 62 deletions src/universal/service/getStates.js
Original file line number Diff line number Diff line change
@@ -1,69 +1,31 @@
import omit from 'lodash/omit';
import camelCase from 'lodash/camelCase';

const blacklistFunctionName = [
'getServerSideProps',
'getServicesWithMultiple',
'getInitialStateWithMultiple',
'setDependencies',
'setSeoState',
'setRedirection',
'setPageData'
];

const getCustomSetters = (component, context, data) => {
const pattern = new RegExp(`^set`);
const functions = omit(component, blacklistFunctionName);
const getResponseData = (component, context, data) => {
let result = {};

Object.entries(functions).forEach(entity => {
const [name, method] = entity;
const isValidName = pattern.test(name);
if (isValidName) {
const propertyName = camelCase(name.replace(pattern, ''));
let value = null;
if (typeof method === 'function') {
value = method(context, data);
} else {
value = method;
}

if (value) {
result = {
...result,
[propertyName]: value
};
}
if (component?.setResponseData) {
if (typeof component.setResponseData === 'function') {
result = component.setResponseData(context, data);
} else {
result = component.setResponseData;
}
});
}

return result;
};

const getStates = async (component, context, predefinedInitialState) => {
const initialState = predefinedInitialState || { data: {} };
let subComponentFiles = [];
let seoState = {};
let responseOptions = {};
let dependencies = [];
let redirection = null;

if (component.setDependencies) {
dependencies = component.setDependencies(context);
}
const responseData = getResponseData(component, context, initialState.data);

if (context.isWithoutState) {
return { initialState, seoState, dependencies, subComponentFiles, responseOptions };
return { initialState, subComponentFiles, responseOptions, ...responseData };
}

if (!predefinedInitialState && component.getServerSideProps) {
if (!predefinedInitialState && component?.getServerSideProps) {
initialState.data = await component.getServerSideProps(context);
}

if (component?.setSeoState) {
seoState = component.setSeoState(initialState?.data) || {};
}

if (initialState?.data?.subComponentFiles) {
subComponentFiles = initialState?.data?.subComponentFiles || [];
}
Expand All @@ -72,24 +34,11 @@ const getStates = async (component, context, predefinedInitialState) => {
responseOptions = initialState?.data?.responseOptions || {};
}

if (component.setRedirection) {
redirection = component.setRedirection(context, initialState.data);
}

if (component.setPageData) {
redirection = component.setPageData(context, initialState.data);
}

const setters = getCustomSetters(component, context, initialState.data);

return {
initialState,
seoState,
subComponentFiles,
responseOptions,
dependencies,
redirection,
...setters
...responseData
};
};

Expand Down
11 changes: 10 additions & 1 deletion src/universal/utils/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,20 @@ const SERVICES = Object.freeze(
}, {})
);

const BLACKLIST_OUTPUT = [
'componentName',
'fullWidth',
'isMobileComponent',
'isPreviewQuery',
'responseOptions'
];

export {
HTTP_STATUS_CODES,
WINDOW_GLOBAL_PARAMS,
JSON_CONTENT_TYPE,
CONTENT_TYPE_HEADER,
REQUEST_TYPES_WITH_BODY,
SERVICES
SERVICES,
BLACKLIST_OUTPUT
};

0 comments on commit 6482ec9

Please sign in to comment.