Skip to content

Commit

Permalink
Adds support for publicPath to enable serving assets from different…
Browse files Browse the repository at this point in the history
… locations. (#299)

Summary:
<!-- Thanks for submitting a pull request! Please provide enough information so that others can review your pull request. The two fields below are mandatory. -->

**Summary**

<!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? -->

This is an updated followup from #280, which we would still need to address the following assumptions about `/assets/`:

https://github.com/facebook/metro/blob/e7deea19001d903a47868bb1fe6456bdc4b585e7/packages/metro/src/Server.js#L332

https://github.com/facebook/metro/blob/e7deea19001d903a47868bb1fe6456bdc4b585e7/packages/metro/src/Server.js#L379

https://github.com/facebook/metro/blob/e7deea19001d903a47868bb1fe6456bdc4b585e7/packages/metro/src/Server.js#L385

As pointed out by gdborton, there isn't currently a way to make this a configurable option. I am not certain how to find a proper workaround for `processRequest`.

We found a temporary solution to have our express app pick up serving the assets from a publicPath as a middleware. But the change in this PR is still necessary to get it working fully.

**Test plan**

<!-- Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes UI. -->

- Will add tests once we figure out a comprehensive solution
Pull Request resolved: #299

Reviewed By: rafeca

Differential Revision: D12939229

Pulled By: mjesun

fbshipit-source-id: 769c23468c5ac434f8319e5e7caaf46dd6453f2d
  • Loading branch information
gdborton authored and facebook-github-bot committed Nov 9, 2018
1 parent 7b89dcd commit ea980fd
Show file tree
Hide file tree
Showing 10 changed files with 88 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ Object {
"minifierPath": "metro-minify-uglify",
"optimizationSizeLimit": 153600,
"postMinifyProcess": [Function],
"publicPath": "/assets",
"transformVariants": Object {
"default": Object {},
},
Expand Down Expand Up @@ -230,6 +231,7 @@ Object {
"minifierPath": "metro-minify-uglify",
"optimizationSizeLimit": 153600,
"postMinifyProcess": [Function],
"publicPath": "/assets",
"transformVariants": Object {
"default": Object {},
},
Expand Down
1 change: 1 addition & 0 deletions packages/metro-config/src/configTypes.flow.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ type TransformerConfigT = {|
postMinifyProcess: PostMinifyProcess,
transformVariants: TransformVariants,
workerPath: string,
publicPath: string,
|};

type MetalConfigT = {|
Expand Down
1 change: 1 addition & 0 deletions packages/metro-config/src/convertConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ async function convertOldToNew({
? transformVariants()
: defaultConfig.transformer.transformVariants,
workerPath: getWorkerPath(),
publicPath: '/assets',
},

reporter,
Expand Down
1 change: 1 addition & 0 deletions packages/metro-config/src/defaults/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ const getDefaultValues = (projectRoot: ?string): ConfigT => ({
postMinifyProcess: x => x,
transformVariants: {default: {}},
workerPath: 'metro/src/DeltaBundler/Worker',
publicPath: '/assets',
},
cacheStores: [
new FileStore({
Expand Down
3 changes: 2 additions & 1 deletion packages/metro/src/Assets.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,9 @@ async function getAssetData(
localPath: string,
assetDataPlugins: $ReadOnlyArray<string>,
platform: ?string = null,
publicPath: string,
): Promise<AssetData> {
let assetUrlPath = path.join('/assets', path.dirname(localPath));
let assetUrlPath = path.join(publicPath, path.dirname(localPath));

// On Windows, change backslashes to slashes to get proper URL path from file path.
if (path.sep === '\\') {
Expand Down
2 changes: 2 additions & 0 deletions packages/metro/src/DeltaBundler/Serializers/getAssets.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ type Options = {|
assetPlugins: $ReadOnlyArray<string>,
platform: ?string,
projectRoot: string,
publicPath: string,
|};

async function getAssets(
Expand All @@ -44,6 +45,7 @@ async function getAssets(
path.relative(options.projectRoot, module.path),
options.assetPlugins,
options.platform,
options.publicPath,
),
);
}
Expand Down
3 changes: 3 additions & 0 deletions packages/metro/src/JSTransformer/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ type BabelTransformerOptions = $ReadOnly<{
minify: boolean,
platform: ?string,
projectRoot: string,
publicPath: string,
}>;

export type BabelTransformerArgs = $ReadOnly<{|
Expand Down Expand Up @@ -88,6 +89,7 @@ export type JsTransformerConfig = $ReadOnly<{|
minifierConfig: MinifierConfig,
minifierPath: string,
optimizationSizeLimit: number,
publicPath: string,
|}>;

export type CustomTransformOptions = {[string]: mixed, __proto__: null};
Expand Down Expand Up @@ -184,6 +186,7 @@ class JsTransformer {
// is used by other tooling, and this would affect it.
inlineRequires: false,
projectRoot: this._projectRoot,
publicPath: this._config.publicPath,
},
plugins: [],
src: sourceCode,
Expand Down
2 changes: 2 additions & 0 deletions packages/metro/src/Server.js
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ class Server {
assetPlugins: this._config.transformer.assetPlugins,
platform: transformOptions.platform,
projectRoot: this._config.projectRoot,
publicPath: this._config.transformer.publicPath,
});
}

Expand Down Expand Up @@ -802,6 +803,7 @@ class Server {
processModuleFilter: this._config.serializer.processModuleFilter,
assetPlugins: this._config.transformer.assetPlugins,
platform: transformOptions.platform,
publicPath: this._config.transformer.publicPath,
projectRoot: this._config.projectRoot,
});
},
Expand Down
83 changes: 73 additions & 10 deletions packages/metro/src/__tests__/Assets-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,13 @@ describe('getAssetData', () => {
'b@4.5x.png': 'b4.5 image',
});

return getAssetData('/root/imgs/b.png', 'imgs/b.png', []).then(data => {
return getAssetData(
'/root/imgs/b.png',
'imgs/b.png',
[],
null,
'/assets',
).then(data => {
expect(data).toEqual(
expect.objectContaining({
__packager_asset: true,
Expand Down Expand Up @@ -153,7 +159,13 @@ describe('getAssetData', () => {
'b@4.5x.jpg': 'b4.5 image',
});

const data = await getAssetData('/root/imgs/b.jpg', 'imgs/b.jpg', []);
const data = await getAssetData(
'/root/imgs/b.jpg',
'imgs/b.jpg',
[],
null,
'/assets',
);

expect(data).toEqual(
expect.objectContaining({
Expand All @@ -173,6 +185,40 @@ describe('getAssetData', () => {
);
});

it('respects `options.publicPath` for output httpServerLocation', async () => {
writeImages({
'b@1x.jpg': 'b1 image',
'b@2x.jpg': 'b2 image',
'b@4x.jpg': 'b4 image',
'b@4.5x.jpg': 'b4.5 image',
});

const data = await getAssetData(
'/root/imgs/b.jpg',
'imgs/b.jpg',
[],
null,
'/public_paths/foo-boar/',
);

expect(data).toEqual(
expect.objectContaining({
__packager_asset: true,
type: 'jpg',
name: 'b',
scales: [1, 2, 4, 4.5],
fileSystemLocation: '/root/imgs',
httpServerLocation: '/public_paths/foo-boar/imgs',
files: [
'/root/imgs/b@1x.jpg',
'/root/imgs/b@2x.jpg',
'/root/imgs/b@4x.jpg',
'/root/imgs/b@4.5x.jpg',
],
}),
);
});

it('loads and runs asset plugins', async () => {
jest.mock(
'mockPlugin1',
Expand Down Expand Up @@ -206,10 +252,13 @@ describe('getAssetData', () => {
'b@3x.png': 'b3 image',
});

const data = await getAssetData('/root/imgs/b.png', 'imgs/b.png', [
'mockPlugin1',
'asyncMockPlugin2',
]);
const data = await getAssetData(
'/root/imgs/b.png',
'imgs/b.png',
['mockPlugin1', 'asyncMockPlugin2'],
null,
'/assets',
);

expect(data).toEqual(
expect.objectContaining({
Expand Down Expand Up @@ -247,21 +296,35 @@ describe('getAssetData', () => {
hash.update(fs.readFileSync(path.join('/root/imgs', name), 'utf8'));
}

expect(await getAssetData('/root/imgs/b.jpg', 'imgs/b.jpg', [])).toEqual(
expect.objectContaining({hash: hash.digest('hex')}),
);
expect(
await getAssetData(
'/root/imgs/b.jpg',
'imgs/b.jpg',
[],
null,
'/assets',
),
).toEqual(expect.objectContaining({hash: hash.digest('hex')}));
});

it('changes the hash when the passed-in file watcher emits an `all` event', async () => {
const initialData = await getAssetData(
'/root/imgs/b.jpg',
'imgs/b.jpg',
[],
null,
'/assets',
);

fs.writeFileSync('/root/imgs/b@4x.jpg', 'updated data');

const data = await getAssetData('/root/imgs/b.jpg', 'imgs/b.jpg', []);
const data = await getAssetData(
'/root/imgs/b.jpg',
'imgs/b.jpg',
[],
null,
'/assets',
);
expect(data.hash).not.toEqual(initialData.hash);
});
});
Expand Down
1 change: 1 addition & 0 deletions packages/metro/src/assetTransformer.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ async function transform(
filename,
assetDataPlugins,
options.platform,
options.publicPath,
);

return {
Expand Down

0 comments on commit ea980fd

Please sign in to comment.