Skip to content
This repository has been archived by the owner on May 15, 2024. It is now read-only.

feat(host): flag to use req.host instead of localhost #14

Merged
merged 3 commits into from
May 13, 2020
Merged

feat(host): flag to use req.host instead of localhost #14

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
befc28d
feat(host): flag to use req.host instead of localhost
infoxicator May 7, 2020
4ad6e05
fix(snapshots): running using node 12.15.0
infoxicator May 7, 2020
1c5a17c
fix(): conflicts and name change
infoxicator May 11, 2020
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
2 changes: 2 additions & 0 deletions __tests__/__snapshots__/index.spec.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

exports[`one-app-dev-cdn module-map.json extends the remote map with the local map 1`] = `"{\\"key\\":\\"not-used-in-development\\",\\"modules\\":{\\"module-b\\":{\\"node\\":{\\"url\\":\\"http://localhost:3001/static/cdn/module-b/1.0.0/module-b.node.js\\",\\"integrity\\":\\"123\\"},\\"browser\\":{\\"url\\":\\"http://localhost:3001/static/cdn/module-b/1.0.0/module-b.browser.js\\",\\"integrity\\":\\"234\\"},\\"legacyBrowser\\":{\\"url\\":\\"http://localhost:3001/static/cdn/module-b/1.0.0/module-b.legacy.browser.js\\",\\"integrity\\":\\"345\\"}},\\"module-a\\":{\\"node\\":{\\"url\\":\\"https://example.com/cdn/module-a/1.0.0/module-a.node.js\\",\\"integrity\\":\\"123\\"},\\"browser\\":{\\"url\\":\\"https://example.com/cdn/module-a/1.0.0/module-a.browser.js\\",\\"integrity\\":\\"234\\"},\\"legacyBrowser\\":{\\"url\\":\\"https://example.com/cdn/module-a/1.0.0/module-a.legacy.browser.js\\",\\"integrity\\":\\"345\\"}}}}"`;

exports[`one-app-dev-cdn module-map.json extends the remote map with the local map and uses host instead of localhost 1`] = `"{\\"key\\":\\"not-used-in-development\\",\\"modules\\":{\\"module-b\\":{\\"node\\":{\\"url\\":\\"http://0.0.0.0:3001/static/cdn/module-b/1.0.0/module-b.node.js\\",\\"integrity\\":\\"123\\"},\\"browser\\":{\\"url\\":\\"http://0.0.0.0:3001/static/cdn/module-b/1.0.0/module-b.browser.js\\",\\"integrity\\":\\"234\\"},\\"legacyBrowser\\":{\\"url\\":\\"http://0.0.0.0:3001/static/cdn/module-b/1.0.0/module-b.legacy.browser.js\\",\\"integrity\\":\\"345\\"}},\\"module-a\\":{\\"node\\":{\\"url\\":\\"https://example.com/cdn/module-a/1.0.0/module-a.node.js\\",\\"integrity\\":\\"123\\"},\\"browser\\":{\\"url\\":\\"https://example.com/cdn/module-a/1.0.0/module-a.browser.js\\",\\"integrity\\":\\"234\\"},\\"legacyBrowser\\":{\\"url\\":\\"https://example.com/cdn/module-a/1.0.0/module-a.legacy.browser.js\\",\\"integrity\\":\\"345\\"}}}}"`;

exports[`one-app-dev-cdn module-map.json uses the local map overriding the cdn url placeholder with the one-app-dev-cdn url 1`] = `
Object {
"key": "not-used-in-development",
Expand Down
62 changes: 61 additions & 1 deletion __tests__/index.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ describe('one-app-dev-cdn', () => {
useLocalModules,
remoteModuleMapUrl,
appPort,
useHost,
} = {}) => {
process.env.NODE_ENV = nodeEnv;

Expand All @@ -117,12 +118,13 @@ describe('one-app-dev-cdn', () => {
remoteModuleMapUrl,
useLocalModules,
appPort,
useHost,
});
};

const sanitizeModuleMapForSnapshot = moduleMapString => moduleMapString.replace(
/\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}:\d{1,8}/g,
'localhost:3001'
'0.0.0.0:3001'
);

process.env.HTTP_ONE_APP_DEV_CDN_PORT = 3001;
Expand Down Expand Up @@ -287,6 +289,43 @@ describe('one-app-dev-cdn', () => {
},
},
};
return supertest(fcdn)
.get('/module-map.json')
.then((response) => {
expect(response.status).toBe(200);
expect(response.header['content-type']).toMatch(/^application\/json/);
expect(response.text).toEqual(JSON.stringify(modifiedRemoteMap));
expect(got.mock.calls[0]).toContain(remoteModuleMapUrl);
});
});

it('modifies the key property and the module URLs using the req.host instead of localhost', () => {
expect.assertions(4);
const remoteModuleMapUrl = 'https://my-domain.com/map/module-map.json';
const fcdn = setupTest({
appPort: 3000, useLocalModules: false, remoteModuleMapUrl, useHost: true,
});
got.mockReturnJsonOnce(defaultRemoteMap);

const modifiedRemoteMap = {
key: 'not-used-in-development',
modules: {
'module-b': {
node: {
url: 'http://0.0.0.0:3001/static/cdn/module-b/1.0.0/module-b.node.js',
integrity: '123',
},
browser: {
url: 'http://0.0.0.0:3001/static/cdn/module-b/1.0.0/module-b.browser.js',
integrity: '234',
},
legacyBrowser: {
url: 'http://0.0.0.0:3001/static/cdn/module-b/1.0.0/module-b.legacy.browser.js',
integrity: '345',
},
},
},
};
return supertest(fcdn)
.get('/module-map.json')
.then((response) => {
Expand Down Expand Up @@ -318,6 +357,27 @@ describe('one-app-dev-cdn', () => {
});
});

it('extends the remote map with the local map and uses host instead of localhost', () => {
expect.assertions(4);
const remoteModuleMapUrl = 'https://my-domain.com/map/module-map.json';

const fcdn = setupTest({
remoteModuleMapUrl, useLocalModules: true, appPort: 3000, useHost: true,
});
got.mockReturnJsonOnce(defaultRemoteMap);

return supertest(fcdn)
.get('/module-map.json')
.then((response) => {
expect(response.status).toBe(200);
expect(response.header['content-type']).toMatch(/^application\/json/);
expect(
sanitizeModuleMapForSnapshot(response.text)
).toMatchSnapshot();
expect(got.mock.calls[0]).toContain(remoteModuleMapUrl);
});
});

it('does not extend the remote map with the local map when useLocalModules is false', () => {
expect.assertions(4);
const remoteModuleMapUrl = 'https://my-domain.com/map/module-map.json';
Expand Down
5 changes: 3 additions & 2 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export default ({
remoteModuleMapUrl,
useLocalModules,
appPort,
useHost,
}) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

what about adding something like below here. you can probably come up with a better name however

Suggested change
}) => {
}) => {
const hostAddress = useHost ? `http://${req.headers.host}` : `http://localhost:${process.env.HTTP_ONE_APP_DEV_CDN_PORT}`;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

had to put it inside the get for the req to be accessible but Implemented your changes here: 1c5a17c

if (!remoteModuleMapUrl && !useLocalModules) {
throw new Error('remoteModuleMapUrl is a required param when useLocalModules is not true');
Expand Down Expand Up @@ -106,7 +107,7 @@ export default ({
const remoteModuleMap = JSON.parse(r.body);

const { modules } = remoteModuleMap;
const oneAppDevStaticsAddress = `http://localhost:${process.env.HTTP_ONE_APP_DEV_CDN_PORT}/static`;
const oneAppDevStaticsAddress = useHost ? `http://${req.headers.host}/static` : `http://localhost:${process.env.HTTP_ONE_APP_DEV_CDN_PORT}/static`;
Copy link
Contributor

Choose a reason for hiding this comment

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

continued from comment above -

Suggested change
const oneAppDevStaticsAddress = useHost ? `http://${req.headers.host}/static` : `http://localhost:${process.env.HTTP_ONE_APP_DEV_CDN_PORT}/static`;
const oneAppDevStaticsAddress = `${hostAddress}/static`;

this change can also be applied below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1c5a17c


Object.keys(modules).forEach((moduleName) => {
const module = modules[moduleName];
Expand Down Expand Up @@ -141,7 +142,7 @@ export default ({
: {},
(useLocalModules ? getLocalModuleMap({
pathToModulemap: path.join(localDevPublicPath, 'module-map.json'),
oneAppDevCdnAddress: `http://localhost:${process.env.HTTP_ONE_APP_DEV_CDN_PORT}`,
oneAppDevCdnAddress: useHost ? `http://${req.headers.host}}` : `http://localhost:${process.env.HTTP_ONE_APP_DEV_CDN_PORT}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
oneAppDevCdnAddress: useHost ? `http://${req.headers.host}}` : `http://localhost:${process.env.HTTP_ONE_APP_DEV_CDN_PORT}`,
oneAppDevCdnAddress: hostAddress,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1c5a17c

})
.then(JSON.parse) : {}),
])
Expand Down