Skip to content

Commit

Permalink
fix: obtain asset size from first asset in group (#814)
Browse files Browse the repository at this point in the history
* test: refactor to use jest each in assetLoader tests

* fix: get asset size for scaled assets like in metro

* chore: add changeset
  • Loading branch information
jbroma authored Dec 8, 2024
1 parent 31d29b5 commit 592fbe3
Show file tree
Hide file tree
Showing 7 changed files with 20 additions and 45 deletions.
5 changes: 5 additions & 0 deletions .changeset/rude-points-ring.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@callstack/repack": patch
---

Fix how size of a scaled assets is obtained (aligned with metro)
Original file line number Diff line number Diff line change
Expand Up @@ -126,36 +126,9 @@ describe('assetLoader', () => {
'[email protected]'
);

describe('on ios', () => {
describe.each(['ios', 'android'])('on %s', (platform) => {
it('should load and extract asset without scales', async () => {
const { code, volume } = await compileBundle('ios', {
...fixtures,
'./index.js': "export { default } from './__fixtures__/logo.png';",
});
const context: { Export?: { default: Record<string, any> } } = {};
vm.runInNewContext(code, context);

expect(context.Export?.default).toMatchSnapshot();
expect(volume.toTree()).toMatchSnapshot();
});

it('should load and extract asset with scales', async () => {
const { code, volume } = await compileBundle('ios', {
...fixtures,
'./index.js': "export { default } from './__fixtures__/star.png';",
});

const context: { Export?: { default: Record<string, any> } } = {};
vm.runInNewContext(code, context);

expect(context.Export?.default).toMatchSnapshot();
expect(volume.toTree()).toMatchSnapshot();
});
});

describe('on android', () => {
it('should load and extract asset without scales', async () => {
const { code, volume } = await compileBundle('android', {
const { code, volume } = await compileBundle(platform, {
...fixtures,
'./index.js': "export { default } from './__fixtures__/logo.png';",
});
Expand All @@ -168,7 +141,7 @@ describe('assetLoader', () => {
});

it('should load and extract asset with scales', async () => {
const { code, volume } = await compileBundle('android', {
const { code, volume } = await compileBundle(platform, {
...fixtures,
'./index.js': "export { default } from './__fixtures__/star.png';",
});
Expand Down
6 changes: 3 additions & 3 deletions packages/repack/src/loaders/assetsLoader/assetsLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,13 @@ export default async function repackAssetsLoader(
}
: null;

// assets are sorted by scale, in ascending order
const assets = await Promise.all<Asset>(
scaleKeys.map(async (scaleKey) => {
const assetPath = scales[scaleKey];
const isDefault = assetPath === resourcePath;
const isLoaded = assetPath === resourcePath;
// use raw Buffer passed to loader to avoid unnecessary read
const content = isDefault ? assetData : await readFileAsync(assetPath);
const content = isLoaded ? assetData : await readFileAsync(assetPath);

let destination: string;

Expand Down Expand Up @@ -197,7 +198,6 @@ export default async function repackAssetsLoader(

return {
data: content,
default: isDefault,
dimensions,
filename: destination,
scale,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import path from 'node:path';
import dedent from 'dedent';
import type { Asset } from './types';
import { getDefaultAsset } from './utils';
import { getAssetSize } from './utils';

export function convertToRemoteAssets({
assets,
Expand Down Expand Up @@ -29,7 +29,8 @@ export function convertToRemoteAssets({
// works on both unix & windows
const publicPathURL = new URL(path.join(remotePublicPath, assetPath));

const size = getDefaultAsset(assets).dimensions;
const size = getAssetSize(assets);

const asset = JSON.stringify({
name: resourceFilename,
type: resourceExtensionType,
Expand Down
4 changes: 2 additions & 2 deletions packages/repack/src/loaders/assetsLoader/extractAssets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import crypto from 'node:crypto';
import path from 'node:path';
import dedent from 'dedent';
import type { Asset } from './types';
import { getDefaultAsset } from './utils';
import { getAssetSize } from './utils';

export function extractAssets(
{
Expand Down Expand Up @@ -39,7 +39,7 @@ export function extractAssets(
publicPath = path.join(customPublicPath, publicPath);
}

const size = getDefaultAsset(assets).dimensions;
const size = getAssetSize(assets);
const scales = assets.map((asset) => asset.scale);
const hashes = assets.map((asset) =>
crypto.createHash('md5').update(asset.data).digest('hex')
Expand Down
1 change: 0 additions & 1 deletion packages/repack/src/loaders/assetsLoader/types.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
export interface Asset {
data: Buffer;
default: boolean;
dimensions: AssetDimensions | null;
filename: string;
scale: number;
Expand Down
11 changes: 4 additions & 7 deletions packages/repack/src/loaders/assetsLoader/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,10 @@ export function getScaleNumber(scaleKey: string) {
return Number.parseFloat(scaleKey.replace(/[^\d.]/g, ''));
}

/** Default asset is the one with scale that was originally requested in the loader */
export function getDefaultAsset(assets: Asset[]) {
const defaultAsset = assets.find((asset) => asset.default === true);
if (!defaultAsset) {
throw new Error('Malformed assets array - no default asset found');
}
return defaultAsset;
export function getAssetSize(assets: Asset[]) {
// Use first asset for reference as size, just like in metro:
// https://github.com/facebook/metro/blob/main/packages/metro/src/Assets.js#L223
return assets[0].dimensions;
}

export function getAssetDimensions({
Expand Down

0 comments on commit 592fbe3

Please sign in to comment.