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

feat: add support for redirect http code in transition plugin #629

Merged
merged 4 commits into from
Dec 24, 2024
Merged
Show file tree
Hide file tree
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
1 change: 1 addition & 0 deletions .husky/commit-msg
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
npx --no -- commitlint --edit $1
2 changes: 1 addition & 1 deletion .husky/pre-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
npm run format:check
npx lint-staged
6 changes: 6 additions & 0 deletions commitlint.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/**
* @type {import('@commitlint/types').UserConfig}
*/
module.exports = {
extends: ['@commitlint/config-conventional'],
};
4 changes: 2 additions & 2 deletions ilc/client/BundleLoader.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,12 @@ describe('BundleLoader', () => {
},
},
}).getConfig();

configRoot.registryConfiguration = registry;
sinon.stub(configRoot, 'registryConfiguration').value(registry);
});

afterEach(() => {
SystemJs.import.reset();
sinon.stub(configRoot, 'registryConfiguration').restore();
});

describe('preloadApp()', () => {
Expand Down
8 changes: 4 additions & 4 deletions ilc/client/Client.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import Router from './ClientRouter';
import initIlcState from './initIlcState';
import I18n from './i18n';

import GuardManager from './GuardManager';
import TransitionHooksExecutor from './TransitionHooksExecutor';
import ParcelApi from './ParcelApi';
import { BundleLoader } from './BundleLoader';
import { registerApplications } from './registerSpaApps';
Expand Down Expand Up @@ -62,7 +62,7 @@ export class Client {

#router;

#guardManager;
#transitionHooksExecutor;

#urlProcessor;

Expand Down Expand Up @@ -114,7 +114,7 @@ export class Client {
singleSpa,
this.#transitionManager.handlePageTransition.bind(this.#transitionManager),
);
this.#guardManager = new GuardManager(
this.#transitionHooksExecutor = new TransitionHooksExecutor(
this.#router,
this.#pluginManager,
this.#onCriticalInternalError.bind(this),
Expand Down Expand Up @@ -268,7 +268,7 @@ export class Client {
}

#configure() {
addNavigationHook((url) => (this.#guardManager.hasAccessTo(url) ? url : null));
addNavigationHook((url) => (this.#transitionHooksExecutor.shouldNavigate(url) ? url : null));
addNavigationHook((url) => this.#urlProcessor.process(url));

// TODO: window.ILC.importLibrary - calls bootstrap function with props (if supported), and returns exposed API
Expand Down
2 changes: 1 addition & 1 deletion ilc/client/ClientRouter.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import deepmerge from 'deepmerge';
import debug from 'debug';
import EventEmitter from 'eventemitter3';

import Router from '../common/router/Router';
import { Router } from '../common/router/Router';
import * as errors from '../common/router/errors';
import { isSpecialUrl } from 'ilc-sdk/app';
import { triggerAppChange } from './navigationEvents';
Expand Down
17 changes: 8 additions & 9 deletions ilc/client/ClientRouter.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ describe('client router', () => {

const specialRoutes = {
404: {
route: '/404',
next: false,
template: 'errorsTemplate',
slots: {},
Expand All @@ -150,16 +149,15 @@ describe('client router', () => {
specialRoutes,
};

let router, configRoot, originalConfig;
let router, configRoot;

before(() => {
configRoot = getIlcConfigRoot();
originalConfig = configRoot.registryConfiguration;
configRoot.registryConfiguration = registryConfig;
sinon.stub(configRoot, 'registryConfiguration').value(registryConfig);
});

after(() => {
configRoot.registryConfiguration = originalConfig;
sinon.stub(configRoot, 'registryConfiguration').restore();
});

afterEach(() => {
Expand Down Expand Up @@ -685,7 +683,7 @@ describe('client router', () => {
const nonPrimaryKind = 'regular';
const appId = 'mounted_app__at__some_place';
const configRoot = getIlcConfigRoot();
configRoot.registryConfiguration = {
const registryConfiguration = {
...registryConfig,
apps: {
...registryConfig.apps,
Expand All @@ -694,6 +692,7 @@ describe('client router', () => {
},
},
};
sinon.stub(configRoot, 'registryConfiguration').value(registryConfiguration);

router = new ClientRouter(configRoot, {}, undefined, singleSpa, handlePageTransaction, undefined, logger);

Expand Down Expand Up @@ -815,7 +814,7 @@ describe('client router', () => {
};

const configRoot = getIlcConfigRoot();
configRoot.registryConfiguration = customRegistryConfig;
sinon.stub(configRoot, 'registryConfiguration').value(customRegistryConfig);

history.replaceState({}, undefined, '/');
router = new ClientRouter(configRoot, {}, undefined, singleSpa, handlePageTransaction, undefined, logger);
Expand Down Expand Up @@ -869,7 +868,7 @@ describe('client router', () => {
};

const configRoot = getIlcConfigRoot();
configRoot.registryConfiguration = customRegistryConfig;
sinon.stub(configRoot, 'registryConfiguration').value(customRegistryConfig);

history.replaceState({}, undefined, '/');
router = new ClientRouter(configRoot, {}, undefined, singleSpa, handlePageTransaction, undefined, logger);
Expand Down Expand Up @@ -929,7 +928,7 @@ describe('client router', () => {
};

const configRoot = getIlcConfigRoot();
configRoot.registryConfiguration = customRegistryConfig;
sinon.stub(configRoot, 'registryConfiguration').value(customRegistryConfig);

history.replaceState({}, undefined, '/');
router = new ClientRouter(configRoot, {}, undefined, singleSpa, handlePageTransaction, undefined, logger);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import errors from '../common/guard/errors';
import actionTypes from '../common/guard/actionTypes';
import { TransitionHookError } from '../common/transition-hooks/errors';
import { ActionType } from '../common/transition-hooks/ActionType';

export default class GuardManager {
/**
* Executes ILC Transition plugin's hooks
*/
stas-nc marked this conversation as resolved.
Show resolved Hide resolved
export default class TransitionHooksExecutor {
#router;
#transitionHooksPlugin;
#errorHandler;
Expand All @@ -14,7 +17,7 @@ export default class GuardManager {
this.#logger = logger;
}

hasAccessTo(url) {
shouldNavigate(url) {
const route = this.#router.match(url);
// This code is executed before the router change, so current = previous
const prevRoute = this.#router.getCurrentRoute();
Expand Down Expand Up @@ -44,21 +47,21 @@ export default class GuardManager {
navigate: this.#router.navigateToUrl,
});

if (action.type === actionTypes.stopNavigation) {
if (action.type === ActionType.stopNavigation) {
this.#logger.info(
`ILC: Stopped navigation due to the Route Guard with index #${hooks.indexOf(hook)}`,
);
return false;
}

if (action.type === actionTypes.redirect) {
if (action.type === ActionType.redirect) {
// Need to add redirect callback to queued tasks
// because it should be executed after micro tasks that can be added after the end of this method
setTimeout(() => {
this.#logger.info(
`ILC: Redirect from "${route.reqUrl}" to "${
action.newLocation
}" due to the Route Guard with index #${hooks.indexOf(hook)}`,
}" due to the Transition Hook with index #${hooks.indexOf(hook)}`,
);
this.#router.navigateToUrl(action.newLocation);
});
Expand All @@ -68,7 +71,7 @@ export default class GuardManager {
const hookIndex = hooks.indexOf(hook);

this.#errorHandler(
new errors.GuardTransitionHookError({
new TransitionHookError({
message: `An error has occurred while executing "${hookIndex}" transition hook for the following URL: "${url}".`,
data: {
hookIndex,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import chai from 'chai';
import sinon from 'sinon';

import errors from '../common/guard/errors';
import actionTypes from '../common/guard/actionTypes';
import GuardManager from './GuardManager';
import { TransitionHookError } from '../common/transition-hooks/errors';
import { ActionType } from '../common/transition-hooks/ActionType';
import TransitionHooksExecutor from './TransitionHooksExecutor';

describe('GuardManager', () => {
describe('TransitionHooksExecutor', () => {
let clock;

const route = Object.freeze({
Expand Down Expand Up @@ -64,30 +64,30 @@ describe('GuardManager', () => {

describe('should have access to a provided URL', () => {
it('if router does not have a route that matches a provided URL', () => {
const hooks = [sinon.stub().returns({ type: actionTypes.stopNavigation })];
const hooks = [sinon.stub().returns({ type: ActionType.stopNavigation })];
transitionHooksPlugin.getTransitionHooks.returns(hooks);
router.match.returns({ specialRole: 404 });

const guardManager = new GuardManager(router, pluginManager, errorHandler, logger);
const transitionHooksExecutor = new TransitionHooksExecutor(router, pluginManager, errorHandler, logger);

chai.expect(guardManager.hasAccessTo('/router/does/not/have/route')).to.be.true;
chai.expect(transitionHooksExecutor.shouldNavigate('/router/does/not/have/route')).to.be.true;
});

it(`if none of hooks returns "${actionTypes.stopNavigation}" or "${actionTypes.redirect}" action types`, () => {
it(`if none of hooks returns "${ActionType.stopNavigation}" or "${ActionType.redirect}" action types`, () => {
const url = '/every/hook/does/not/return/stop/navigation';
const hooks = [
sinon.stub().returns({ type: actionTypes.continue }),
sinon.stub().returns({ type: actionTypes.continue }),
sinon.stub().returns({ type: ActionType.continue }),
sinon.stub().returns({ type: ActionType.continue }),
sinon.stub().returns({ type: null }),
sinon.stub().returns({ type: undefined }),
];

transitionHooksPlugin.getTransitionHooks.returns(hooks);
router.match.returns(route);

const guardManager = new GuardManager(router, pluginManager, errorHandler, logger);
const transitionHooksExecutor = new TransitionHooksExecutor(router, pluginManager, errorHandler, logger);

chai.expect(guardManager.hasAccessTo(url)).to.be.true;
chai.expect(transitionHooksExecutor.shouldNavigate(url)).to.be.true;

for (const hook of hooks) {
sinon.assert.calledOnceWithExactly(hook, {
Expand All @@ -104,7 +104,7 @@ describe('GuardManager', () => {
const error = new Error('Hi there! I am an error. So, should be shown 500 error page in this case');
const url = '/some/hook/returns/stop/navigation';
const hooks = [
sinon.stub().returns({ type: actionTypes.continue }),
sinon.stub().returns({ type: ActionType.continue }),
sinon.stub().throws(error),
sinon.stub().returns({ type: true }),
sinon.stub().returns({ type: 0 }),
Expand All @@ -113,12 +113,12 @@ describe('GuardManager', () => {
transitionHooksPlugin.getTransitionHooks.returns(hooks);
router.match.returns(route);

const guardManager = new GuardManager(router, pluginManager, errorHandler, logger);
const transitionHooksExecutor = new TransitionHooksExecutor(router, pluginManager, errorHandler, logger);

chai.expect(guardManager.hasAccessTo(url)).to.be.false;
chai.expect(transitionHooksExecutor.shouldNavigate(url)).to.be.false;
sinon.assert.calledOnce(errorHandler);
chai.expect(errorHandler.getCall(0).args[0]).to.have.property('cause', error);
chai.expect(errorHandler.getCall(0).args[0]).to.be.instanceOf(errors.GuardTransitionHookError);
chai.expect(errorHandler.getCall(0).args[0]).to.be.instanceOf(TransitionHookError);
chai.expect(errorHandler.getCall(0).args[0].data).to.be.eql({
hookIndex: 1,
url,
Expand All @@ -138,21 +138,21 @@ describe('GuardManager', () => {
}
});

it(`if some of hooks returns "${actionTypes.stopNavigation}" action type`, () => {
it(`if some of hooks returns "${ActionType.stopNavigation}" action type`, () => {
const url = '/some/hook/returns/stop/navigation';
const hooks = [
sinon.stub().returns({ type: actionTypes.continue }),
sinon.stub().returns({ type: actionTypes.stopNavigation }),
sinon.stub().returns({ type: ActionType.continue }),
sinon.stub().returns({ type: ActionType.stopNavigation }),
sinon.stub().returns({ type: true }),
sinon.stub().returns({ type: 0 }),
];

transitionHooksPlugin.getTransitionHooks.returns(hooks);
router.match.returns(route);

const guardManager = new GuardManager(router, pluginManager, errorHandler, logger);
const transitionHooksExecutor = new TransitionHooksExecutor(router, pluginManager, errorHandler, logger);

chai.expect(guardManager.hasAccessTo(url)).to.be.false;
chai.expect(transitionHooksExecutor.shouldNavigate(url)).to.be.false;
sinon.assert.calledOnceWithExactly(
logger.info,
`ILC: Stopped navigation due to the Route Guard with index #${1}`,
Expand All @@ -171,21 +171,21 @@ describe('GuardManager', () => {
}
});

it(`if some of hooks returns "${actionTypes.redirect}" action type`, async () => {
it(`if some of hooks returns "${ActionType.redirect}" action type`, async () => {
const url = '/some/hook/returns/redirect/navigation';
const hooks = [
sinon.stub().returns({ type: actionTypes.continue }),
sinon.stub().returns({ type: actionTypes.redirect, newLocation: url }),
sinon.stub().returns({ type: ActionType.continue }),
sinon.stub().returns({ type: ActionType.redirect, newLocation: url }),
sinon.stub().returns({ type: false }),
sinon.stub().returns({ type: 1 }),
];

transitionHooksPlugin.getTransitionHooks.returns(hooks);
router.match.returns(route);

const guardManager = new GuardManager(router, pluginManager, errorHandler, logger);
const transitionHooksExecutor = new TransitionHooksExecutor(router, pluginManager, errorHandler, logger);

chai.expect(guardManager.hasAccessTo(url)).to.be.false;
chai.expect(transitionHooksExecutor.shouldNavigate(url)).to.be.false;
sinon.assert.notCalled(router.navigateToUrl);

for (const hook of [hooks[0], hooks[1]]) {
Expand All @@ -204,7 +204,7 @@ describe('GuardManager', () => {

sinon.assert.calledWithExactly(
logger.info,
`ILC: Redirect from "${route.reqUrl}" to "${url}" due to the Route Guard with index #${1}`,
`ILC: Redirect from "${route.reqUrl}" to "${url}" due to the Transition Hook with index #${1}`,
);
sinon.assert.calledWithExactly(router.navigateToUrl, url);
});
Expand Down
7 changes: 5 additions & 2 deletions ilc/client/composeAppSlotPairsToRegister.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ import composeAppSlotPairsToRegister from './composeAppSlotPairsToRegister';
import { getIlcConfigRoot } from './configuration/getIlcConfigRoot';

describe('select slots to register', () => {
const configRoot = getIlcConfigRoot();
after(() => {
sinon.stub(configRoot, 'registryConfiguration').restore();
});
it('should select slots without any duplicated apps of slots from provided routes', () => {
const routes = [
{
Expand Down Expand Up @@ -107,10 +111,9 @@ describe('select slots to register', () => {
},
];

const configRoot = getIlcConfigRoot();
const registryConf = getRegistryMock().getConfig();
registryConf.routes = routes;
configRoot.registryConfiguration = registryConf;
sinon.stub(configRoot, 'registryConfiguration').value(registryConf);

const composedAppsJsonified = composeAppSlotPairsToRegister(configRoot).map((item) => item.toJSON());

Expand Down
10 changes: 1 addition & 9 deletions ilc/common/DefaultCacheWrapper.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,7 @@ describe('DefaultCacheWrapper', () => {
});

it('should return the same value in case of concurrent invocation', async () => {
fn.withArgs().callsFake(
() =>
new Promise((resolve) =>
setTimeout(() => {
console.log('CALLED');
return resolve(data);
}, 100),
),
);
fn.withArgs().callsFake(() => new Promise((resolve) => setTimeout(() => resolve(data), 100)));

const [firstValue, secondValue, thirdValue] = await Promise.all([wrappedFn(), wrappedFn(), wrappedFn()]);

Expand Down
5 changes: 0 additions & 5 deletions ilc/common/guard/actionTypes.js

This file was deleted.

Loading
Loading