Skip to content

Commit

Permalink
feat: serve source assets (#702)
Browse files Browse the repository at this point in the history
* feat: enforce naming in development

* feat: add .map type to mimetypes

* refactor: simplify obtaining sourcemaps

* fix: getMimeType type

* refactor: adjust getAsset type

* feat: compiler plugin can serve local assets too

* refactor: revert changes to externally available compiler plugin types

* refactor: revert changes to compilerPlugin

* refactor: dont alter filename in compilerPlugin

* refactor: use inferPlatform for obtaining platform

* refactor: remove redundant enforcement of sourcemap filename

* refactor: naming

* refactor: naming

* refactor: cleanup

* fix: handle missing platform in Compiler instead of compilerPlugin

* chore: ignore dev-server type error for now

* chore: add changeset

* fix: align parseFileUrl after merge

* refactor: extract getMimeType to common

* refactor: align getSourceMap

* fix: align changes for rspack

* fix: align getSource for rspack

* refactor: make the asset regex more maintainable

* test: verify remote-assets are accessible in dev-server

* test: verify source files are accessible in dev-server

* test: add getMimeType tests

* refactor: align return type to match mime-types lib

* fix: include hot-update.json as well
  • Loading branch information
jbroma authored Sep 30, 2024
1 parent 66f2949 commit 495203d
Show file tree
Hide file tree
Showing 14 changed files with 154 additions and 105 deletions.
5 changes: 5 additions & 0 deletions .changeset/happy-pets-yawn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@callstack/repack': minor
---

Enable dev-server to serve source assets alongside build artifacts
12 changes: 12 additions & 0 deletions apps/tester-app/__tests__/start.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@ describe('start command', () => {
'ios/src_asyncChunks_Async_local_tsx.chunk.bundle.map',
'assets/src/miniapp/callstack-dark.png?platform=ios',
`assets/${RELATIVE_REACT_NATIVE_PATH}/Libraries/NewAppScreen/components/logo.png?platform=ios`,
`remote-assets/assets/src/assetsTest/remoteAssets/webpack.png?platform=ios`,
`remote-assets/assets/src/assetsTest/remoteAssets/[email protected]?platform=ios`,
`remote-assets/assets/src/assetsTest/remoteAssets/[email protected]?platform=ios`,
`index.js`,
`src/App.tsx`,
`src/ui/undraw_Developer_activity_re_39tg.svg`,
],
},
{
Expand All @@ -74,6 +80,12 @@ describe('start command', () => {
'android/src_asyncChunks_Async_local_tsx.chunk.bundle.map',
'assets/src/miniapp/callstack-dark.png?platform=android',
`assets/${RELATIVE_REACT_NATIVE_PATH}/Libraries/NewAppScreen/components/logo.png?platform=android`,
`remote-assets/assets/src/assetsTest/remoteAssets/webpack.png?platform=android`,
`remote-assets/assets/src/assetsTest/remoteAssets/[email protected]?platform=android`,
`remote-assets/assets/src/assetsTest/remoteAssets/[email protected]?platform=android`,
`index.js`,
`src/App.tsx`,
`src/ui/undraw_Developer_activity_re_39tg.svg`,
],
},
])(
Expand Down
22 changes: 8 additions & 14 deletions packages/dev-server/src/plugins/compiler/compilerPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ async function compilerPlugin(
},
},
handler: async (request, reply) => {
let file = (request.params as { '*'?: string })['*'];
const filename = (request.params as { '*'?: string })['*'];
let { platform } = request.query as { platform?: string };

if (!file) {
if (!filename) {
// This technically should never happen - this route should not be called if file is missing.
request.log.error(`File was not provided`);
return reply.notFound();
Expand All @@ -34,16 +34,6 @@ async function compilerPlugin(
// to platform query param.
platform = delegate.compiler.inferPlatform?.(request.url) ?? platform;

if (!platform) {
request.log.error('Cannot detect platform');
return reply.badRequest('Cannot detect platform');
}

// If platform happens to be in front of an asset remove it.
if (file.startsWith(`${platform}/`)) {
file = file.replace(`${platform}/`, '');
}

const multipart = reply.asMultipart();

const sendProgress: SendProgress = ({ completed, total }) => {
Expand All @@ -58,11 +48,15 @@ async function compilerPlugin(

try {
const asset = await delegate.compiler.getAsset(
file,
filename,
platform,
sendProgress
);
const mimeType = delegate.compiler.getMimeType(file, platform, asset);
const mimeType = delegate.compiler.getMimeType(
filename,
platform,
asset
);

if (multipart) {
const buffer = Buffer.isBuffer(asset) ? asset : Buffer.from(asset);
Expand Down
4 changes: 2 additions & 2 deletions packages/dev-server/src/plugins/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export interface CompilerDelegate {
*/
getAsset: (
filename: string,
platform: string,
platform: string | undefined,
sendProgress?: SendProgress
) => Promise<string | Buffer>;

Expand All @@ -28,7 +28,7 @@ export interface CompilerDelegate {
*/
getMimeType: (
filename: string,
platform: string,
platform: string | undefined,
data: string | Buffer
) => string;

Expand Down
12 changes: 12 additions & 0 deletions packages/repack/src/commands/common/__tests__/getMimeType.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { getMimeType } from '../getMimeType';

describe('getMimeType', () => {
it('should return correct MIME types for various file extensions', () => {
expect(getMimeType('script.js')).toBe('application/javascript');
expect(getMimeType('main.bundle')).toBe('application/javascript');
expect(getMimeType('main.bundle.map')).toBe('application/json');
expect(getMimeType('hot-update.js')).toBe('application/javascript');
expect(getMimeType('image.png')).toBe('image/png');
expect(getMimeType('unknownfile.unknown')).toBe('text/plain');
});
});
22 changes: 22 additions & 0 deletions packages/repack/src/commands/common/getMimeType.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import mimeTypes from 'mime-types';

/**
* Get the MIME type for a given filename.
*
* Note: The `mime-types` library currently uses 'application/javascript' for JavaScript files,
* but 'text/javascript' is more widely recognized and standard.
*
* @param {string} filename - The name of the file to get the MIME type for.
* @returns {string} - The MIME type of the file.
*/
export function getMimeType(filename: string) {
if (filename.endsWith('.bundle')) {
return 'application/javascript';
}

if (filename.endsWith('.map')) {
return 'application/json';
}

return mimeTypes.lookup(filename) || 'text/plain';
}
1 change: 1 addition & 0 deletions packages/repack/src/commands/common/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
export * from './adaptFilenameToPlatform';
export * from './getConfigFilePath';
export * from './getEnvOptions';
export * from './getMimeType';
export * from './loadConfig';
export * from './runAdbReverse';
export * from './parseFileUrl';
Expand Down
6 changes: 4 additions & 2 deletions packages/repack/src/commands/common/parseFileUrl.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
export function parseFileUrl(fileUrl: string) {
const { pathname, searchParams } = new URL(fileUrl);
export function parseFileUrl(fileUrl: string, base?: string) {
const url = new URL(fileUrl, base);
const { pathname, searchParams } = url;

let platform = searchParams.get('platform');
let filename = pathname;

Expand Down
16 changes: 16 additions & 0 deletions packages/repack/src/commands/consts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,19 @@ export const DEFAULT_RSPACK_CONFIG_LOCATIONS = [
'rspack.config.cjs',
'rspack.config.js',
];

/**
* Dev Server supported asset types.
*
* These are the types of assets that will be served from the compiler output
* instead of the local filesystem.
*/
export const DEV_SERVER_ASSET_TYPES = new RegExp(
[
'\\.bundle$',
'\\.map$',
'\\.hot-update\\.js(on)?$',
'^assets',
'^remote-assets',
].join('|')
);
55 changes: 23 additions & 32 deletions packages/repack/src/commands/rspack/Compiler.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import fs from 'node:fs';
import path from 'node:path';
import memfs from 'memfs';
import mimeTypes from 'mime-types';
import { Configuration, rspack } from '@rspack/core';
import type {
MultiCompiler,
Expand All @@ -13,6 +12,7 @@ import type { Reporter } from '../../logging';
import type { HMRMessageBody } from '../../types';
import type { StartCliOptions } from '../types';
import { adaptFilenameToPlatform, getEnvOptions, loadConfig } from '../common';
import { DEV_SERVER_ASSET_TYPES } from '../consts';
import type { CompilerAsset, MultiWatching } from './types';

export class Compiler {
Expand Down Expand Up @@ -232,37 +232,42 @@ export class Compiler {

async getSource(
filename: string,
platform?: string
platform: string | undefined
): Promise<string | Buffer> {
/**
* TODO refactor this part
*
* This code makes an assumption that filename ends with .bundle
* but this can be changed by the user, so is prone to breaking
* In reality, it's not that big a deal. This part is within a dev server
* so we might override & enforce the format for the purpose of development
*/
if (/\.(bundle|hot-update\.js)/.test(filename) && platform) {
return (await this.getAsset(filename, platform)).data;
if (DEV_SERVER_ASSET_TYPES.test(filename)) {
if (!platform) {
throw new Error(`Cannot detect platform for ${filename}`);
}
const asset = await this.getAsset(filename, platform);
return asset.data;
}

return fs.promises.readFile(
path.join(this.cliOptions.config.root, filename),
'utf8'
);
try {
const filePath = path.join(this.cliOptions.config.root, filename);
const source = await fs.promises.readFile(filePath, 'utf8');
return source;
} catch {
throw new Error(`File ${filename} not found`);
}
}

async getSourceMap(
filename: string,
platform: string
platform: string | undefined
): Promise<string | Buffer> {
if (!platform) {
throw new Error(
`Cannot determine platform for source map of ${filename}`
);
}

try {
const { info } = await this.getAsset(filename, platform);
let sourceMapFilename = info.related?.sourceMap;

if (!sourceMapFilename) {
throw new Error(
`No source map associated with ${filename} for ${platform}`
`Cannot determine source map filename for ${filename} for ${platform}`
);
}

Expand All @@ -277,20 +282,6 @@ export class Compiler {
}
}

getMimeType(filename: string) {
/**
* TODO potentially refactor
*
* same as in getSource, this part is prone to breaking
* if the user changes the filename format
*/
if (filename.endsWith('.bundle')) {
return 'text/javascript';
}

return mimeTypes.lookup(filename) || 'text/plain';
}

getHmrBody(platform: string): HMRMessageBody | null {
const stats = this.statsCache[platform];
if (!stats) {
Expand Down
30 changes: 8 additions & 22 deletions packages/repack/src/commands/rspack/start.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { URL } from 'node:url';
import colorette from 'colorette';
import { Config } from '@react-native-community/cli-types';
import packageJson from '../../../package.json';
Expand All @@ -12,6 +11,7 @@ import {
import { DEFAULT_HOSTNAME, DEFAULT_PORT } from '../consts';
import { StartArguments, StartCliOptions } from '../types';
import {
getMimeType,
getRspackConfigFilePath,
parseFileUrl,
runAdbReverse,
Expand Down Expand Up @@ -125,17 +125,14 @@ export async function start(

return {
compiler: {
getAsset: async (filename, platform) =>
(await compiler.getAsset(filename, platform)).data,
getMimeType: (filename) => compiler.getMimeType(filename),
getAsset: (filename, platform) => {
const parsedUrl = parseFileUrl(filename, 'file:///');
return compiler.getSource(parsedUrl.filename, platform);
},
getMimeType: (filename) => getMimeType(filename),
inferPlatform: (uri) => {
const url = new URL(uri, 'protocol://domain');
if (!url.searchParams.get('platform')) {
const [, platform] = /^\/(.+)\/.+$/.exec(url.pathname) ?? [];
return platform;
}

return undefined;
const { platform } = parseFileUrl(uri, 'file:///');
return platform;
},
},
symbolicator: {
Expand All @@ -144,18 +141,7 @@ export async function start(
return compiler.getSource(filename, platform);
},
getSourceMap: (fileUrl) => {
// TODO Align this with output.hotModuleUpdateChunkFilename
if (fileUrl.endsWith('.hot-update.js')) {
const { pathname } = new URL(fileUrl);
const [platform, filename] = pathname.split('/').filter(Boolean);
return compiler.getSourceMap(filename, platform);
}

const { filename, platform } = parseFileUrl(fileUrl);
if (!platform) {
throw new Error('Cannot infer platform for file URL');
}

return compiler.getSourceMap(filename, platform);
},
shouldIncludeFrame: (frame) => {
Expand Down
Loading

0 comments on commit 495203d

Please sign in to comment.