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

fix(module-loader-commonjs): avoid using fs to require commonjs module #533

Merged
merged 2 commits into from
Jul 30, 2019
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
29 changes: 27 additions & 2 deletions docs/guides/feature-app-in-feature-app.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,13 @@ added:
```

And the integrator must provide `@feature-hub/react` and `react` as shared
dependencies:
dependencies.

On the client:

```js
import {defineExternals} from '@feature-hub/module-loader-amd';
import {createFeatureHub} from '@feature-hub/core';
import {defineExternals, loadAmdModule} from '@feature-hub/module-loader-amd';
import * as FeatureHubReact from '@feature-hub/react';
import * as React from 'react';
```
Expand All @@ -52,6 +55,28 @@ defineExternals({
react: React,
'@feature-hub/react': FeatureHubReact
});

const {featureAppManager} = createFeatureHub('acme:integrator', {
moduleLoader: loadAmdModule
});
```

On the server:

```js
import {createFeatureHub} from '@feature-hub/core';
import {createCommonJsModuleLoader} from '@feature-hub/module-loader-commonjs';
import * as FeatureHubReact from '@feature-hub/react';
import * as React from 'react';
```

```js
const {featureAppManager} = createFeatureHub('acme:integrator', {
moduleLoader: createCommonJsModuleLoader({
react: React,
'@feature-hub/react': FeatureHubReact
})
});
```

[react-feature-app-container]:
Expand Down
27 changes: 23 additions & 4 deletions docs/guides/sharing-npm-dependencies.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,10 @@ using the `defineExternals` function:
import {createFeatureHub} from '@feature-hub/core';
import {defineExternals, loadAmdModule} from '@feature-hub/module-loader-amd';
import * as React from 'react';
import Loadable from 'react-loadable';
```

```js
defineExternals({react: React, 'react-loadable': Loadable});
defineExternals({react: React});

const {featureAppManager} = createFeatureHub('acme:integrator', {
moduleLoader: loadAmdModule
Expand All @@ -38,15 +37,35 @@ defining `react` as external in a webpack config would look like this:
```

On the server, when Feature Apps are [bundled as CommonJS modules][serversrc],
the integrator needs to make sure that the externals are provided as node
modules that can be loaded with `require()`.
the integrator either

1. needs to make sure that the externals are provided as node modules that can
be loaded with `require()`, or

1. if the integrator code is bundled and no node modules are available during
runtime, the integrator can provide shared npm dependencies to Feature Apps
using the `createCommonJsModuleLoader` function of the
[`@feature-hub/module-loader-commonjs`][module-loader-commonjs-api] package:

```js
import {createFeatureHub} from '@feature-hub/core';
import {createCommonJsModuleLoader} from '@feature-hub/module-loader-commonjs';
import * as React from 'react';
```

```js
const {featureAppManager} = createFeatureHub('acme:integrator', {
moduleLoader: createCommonJsModuleLoader({react: React})
});
```

To validate the external dependencies that are [required by Feature
Apps][feature-app-dependencies] against the shared npm dependencies that are
provided by the integrator, [the `ExternalsValidator` can be
used][validating-externals].

[module-loader-amd-api]: /@feature-hub/modules/module_loader_amd.html
[module-loader-commonjs-api]: /@feature-hub/modules/module_loader_commonjs.html
[amd-module-loader-demo]:
https://github.com/sinnerschrader/feature-hub/tree/master/packages/demos/src/module-loader-amd
[serversrc]: /docs/guides/integrating-the-feature-hub#serversrc
Expand Down
6 changes: 2 additions & 4 deletions packages/module-loader-commonjs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,10 @@
"typings": "lib/cjs/index.d.ts",
"dependencies": {
"@feature-hub/core": "^2.2.0",
"fs-extra": "^8.0.0",
"node-fetch": "^2.3.0"
"node-fetch": "^2.6.0"
},
"devDependencies": {
"@types/fs-extra": "^8.0.0",
"@types/node-fetch": "^2.1.4"
"@types/node-fetch": "^2.3.7"
},
"publishConfig": {
"access": "public"
Expand Down
28 changes: 25 additions & 3 deletions packages/module-loader-commonjs/src/__tests__/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,18 @@
* @jest-environment node
*/

import {createCommonJsModuleLoader, loadCommonJsModule} from '..';

let mockResponse: string;

// tslint:disable promise-function-async
jest.mock('node-fetch', () => () =>
Promise.resolve({
buffer: () => Promise.resolve(Buffer.from(mockResponse))
text: () => Promise.resolve(Buffer.from(mockResponse))
})
);
// tslint:enable promise-function-async

import {loadCommonJsModule} from '..';

describe('loadCommonJsModule (on Node.js)', () => {
it('when a module is fetched successfully', async () => {
const url = 'http://example.com/test.js';
Expand All @@ -31,3 +31,25 @@ describe('loadCommonJsModule (on Node.js)', () => {
expect(loadedModule).toEqual(expectedModule);
});
});

describe('createCommonJsModuleLoader', () => {
it('creates a CommonJS module loader with custom-defined externals', async () => {
const url = 'http://example.com/test.js';

mockResponse = `
var foo = require('foo');
module.exports = {
default: {test: foo()}
};
`;

const loadCommonJsModuleWithExternals = createCommonJsModuleLoader({
foo: () => 42
});

const loadedModule = await loadCommonJsModuleWithExternals(url);
const expectedModule = {default: {test: 42}};

expect(loadedModule).toEqual(expectedModule);
});
});
60 changes: 25 additions & 35 deletions packages/module-loader-commonjs/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,42 +1,32 @@
import {ModuleLoader} from '@feature-hub/core';
import crypto from 'crypto';
import fs from 'fs-extra';
import fetch from 'node-fetch';
import path from 'path';

// This must be a local directory to make node module resolution possible.
const tmpDirname = path.join(__dirname, '.tmp');

function createHash(data: string): string {
return crypto
.createHash('md5')
.update(data)
.digest('hex');
}

async function writeFile(url: string, data: Buffer): Promise<string> {
/* istanbul ignore next */
if (!(await fs.pathExists(tmpDirname))) {
await fs.mkdirp(tmpDirname);
}

const filename = path.join(tmpDirname, `${createHash(url)}.js`);

await fs.writeFile(filename, data);

return filename;
export interface Externals {
readonly [externalName: string]: unknown;
}

async function requireAsync(url: string): Promise<unknown> {
const response = await fetch(url);
const data = await response.buffer();
const filename = await writeFile(url, data);

// Make sure no stale version of the module is loaded from the require cache!
// tslint:disable-next-line:no-dynamic-delete
delete require.cache[require.resolve(filename)];

return require(filename);
export function createCommonJsModuleLoader(
externals: Externals = {}
): ModuleLoader {
return async (url: string): Promise<unknown> => {
const response = await fetch(url);
const source = await response.text();
const mod = {exports: {}};

// tslint:disable-next-line:function-constructor
Function(
'module',
'exports',
'require',
`${source}
//# sourceURL=${url}`
)(mod, mod.exports, (dep: string) =>
// tslint:disable-next-line:no-eval https://stackoverflow.com/a/41063795/10385541
externals.hasOwnProperty(dep) ? externals[dep] : eval('require')(dep)
);

return mod.exports;
};
}

export const loadCommonJsModule: ModuleLoader = async url => requireAsync(url);
export const loadCommonJsModule: ModuleLoader = createCommonJsModuleLoader();
19 changes: 6 additions & 13 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2194,13 +2194,6 @@
"@types/express-serve-static-core" "*"
"@types/serve-static" "*"

"@types/fs-extra@^8.0.0":
version "8.0.0"
resolved "https://registry.yarnpkg.com/@types/fs-extra/-/fs-extra-8.0.0.tgz#d3e2c313ca29f95059f198dd60d1f774642d4b25"
integrity sha512-bCtL5v9zdbQW86yexOlXWTEGvLNqWxMFyi7gQA7Gcthbezr2cPSOb8SkESVKA937QD5cIwOFLDFt0MQoXOEr9Q==
dependencies:
"@types/node" "*"

"@types/get-port@^4.0.0":
version "4.2.0"
resolved "https://registry.yarnpkg.com/@types/get-port/-/get-port-4.2.0.tgz#4fc44616c737d37d3ee7926d86fa975d0afba5e4"
Expand Down Expand Up @@ -2288,10 +2281,10 @@
resolved "https://registry.yarnpkg.com/@types/minimatch/-/minimatch-3.0.3.tgz#3dca0e3f33b200fc7d1139c0cd96c1268cadfd9d"
integrity sha512-tHq6qdbT9U1IRSGf14CL0pUlULksvY9OZ+5eEgl1N7t+OA3tGvNpxJCzuKQlsNgCVwbAs670L1vcVQi8j9HjnA==

"@types/node-fetch@^2.1.4":
version "2.3.7"
resolved "https://registry.yarnpkg.com/@types/node-fetch/-/node-fetch-2.3.7.tgz#b7212e895100f8642dbdab698472bab5f3c1d2f1"
integrity sha512-+bKtuxhj/TYSSP1r4CZhfmyA0vm/aDRQNo7vbAgf6/cZajn0SAniGGST07yvI4Q+q169WTa2/x9gEHfJrkcALw==
"@types/node-fetch@^2.3.7":
version "2.5.0"
resolved "https://registry.yarnpkg.com/@types/node-fetch/-/node-fetch-2.5.0.tgz#1c55616a4591bdd15a389fbd0da4a55b9502add5"
integrity sha512-TLFRywthBgL68auWj+ziWu+vnmmcHCDFC/sqCOQf1xTz4hRq8cu79z8CtHU9lncExGBsB8fXA4TiLDLt6xvMzw==
dependencies:
"@types/node" "*"

Expand Down Expand Up @@ -6243,7 +6236,7 @@ fs-extra@^7.0.0:
jsonfile "^4.0.0"
universalify "^0.1.0"

fs-extra@^8.0.0, fs-extra@^8.1.0:
fs-extra@^8.1.0:
version "8.1.0"
resolved "https://registry.yarnpkg.com/fs-extra/-/fs-extra-8.1.0.tgz#49d43c45a88cd9677668cb7be1b46efdb8d2e1c0"
integrity sha512-yhlQgA6mnOJUKOsRUFsgJdQCvkKhcz8tlZG5HBQfReYZy46OwLcY+Zia0mtdHsOo9y/hP+CxMN0TU9QxoOtG4g==
Expand Down Expand Up @@ -9414,7 +9407,7 @@ node-fetch@^1.0.1:
encoding "^0.1.11"
is-stream "^1.0.1"

node-fetch@^2.3.0, node-fetch@^2.5.0:
node-fetch@^2.3.0, node-fetch@^2.5.0, node-fetch@^2.6.0:
version "2.6.0"
resolved "https://registry.yarnpkg.com/node-fetch/-/node-fetch-2.6.0.tgz#e633456386d4aa55863f676a7ab0daa8fdecb0fd"
integrity sha512-8dG4H5ujfvFiqDmVu9fQ5bOHUC15JMjMY/Zumv26oOvvVJjM67KF8koCWIabKQ1GJIa9r2mMZscBq/TbdOcmNA==
Expand Down