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: assets in transitive dependencies #661

Merged
merged 6 commits into from
Oct 24, 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
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,5 @@ dist/
!**/fixtures/node_modules/
.vscode/
.DS_Store
.coverage_output
.coverage_output
fixtures/node_modules/
5 changes: 4 additions & 1 deletion e2e/react_native_0_60x/__tests__/bundle.android.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import createBundleTestSuite from '../../utils/createBundleTestSuite';

createBundleTestSuite('react_native_with_haul_0_60x', 'android');
createBundleTestSuite('react_native_with_haul_0_60x', 'android', {
testRamBundle: true,
checkAssets: true,
});
5 changes: 4 additions & 1 deletion e2e/react_native_0_60x/__tests__/bundle.ios.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import createBundleTestSuite from '../../utils/createBundleTestSuite';

createBundleTestSuite('react_native_with_haul_0_60x', 'ios');
createBundleTestSuite('react_native_with_haul_0_60x', 'ios', {
testRamBundle: true,
checkAssets: true,
});
21 changes: 13 additions & 8 deletions e2e/utils/bundle.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,25 @@
// @ts-ignore
import { runHaulSync } from './runHaul';
import path from 'path';
import rimraf from 'rimraf';

export function bundleForPlatform(projectDir: string, platform: string) {
export function bundleForPlatform(
projectDir: string,
platform: string,
{ ramBundle }: { ramBundle?: boolean } = {}
) {
const bundlePath = path.resolve(
projectDir,
'dist',
platform === 'ios' ? 'index.jsbundle' : 'index.android.bundle'
);
const { stdout } = runHaulSync(projectDir, [
'bundle',
ramBundle ? 'ram-bundle' : 'bundle',
'--platform',
platform,
'--bundle-output',
bundlePath,
'--assets-dest',
path.resolve(projectDir, 'dist'),
]);

if (stdout.match(/(error ▶︎ |ERROR)/g)) {
Expand All @@ -21,9 +29,6 @@ export function bundleForPlatform(projectDir: string, platform: string) {
return bundlePath;
}

export function cleanup(projectDir: string, platform: string) {
const filename =
platform === 'ios' ? 'index.jsbundle' : 'index.android.bundle';
rimraf.sync(path.resolve(projectDir, filename));
rimraf.sync(path.resolve(projectDir, `${filename}.map`));
export function cleanup(projectDir: string) {
rimraf.sync(path.resolve(projectDir, 'dist'));
}
69 changes: 66 additions & 3 deletions e2e/utils/createBundleTestSuite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,79 @@ import { bundleForPlatform, cleanup } from './bundle';

export default function createBundleTestSuite(
projectName: string,
platform: string
platform: string,
{
testRamBundle,
checkAssets,
}: { testRamBundle?: boolean; checkAssets?: boolean } = {}
) {
const PROJECT_FIXTURE = path.join(__dirname, '../../fixtures', projectName);

beforeAll(() => installDeps(PROJECT_FIXTURE));
beforeEach(() => cleanup(PROJECT_FIXTURE, platform));
afterAll(() => cleanup(PROJECT_FIXTURE, platform));
beforeEach(() => cleanup(PROJECT_FIXTURE));
afterAll(() => cleanup(PROJECT_FIXTURE));

function assertAssets(bundlePath: string) {
if (checkAssets) {
const dist = path.dirname(bundlePath);
if (platform === 'android') {
expect(
fs.existsSync(
path.join(dist, 'drawable-mdpi/node_modules_foo_asset.png')
)
).toBe(true);
expect(
fs.existsSync(
path.join(dist, 'drawable-mdpi/node_modules_baz_asset.png')
)
).toBe(true);
expect(
fs.existsSync(
path.join(
dist,
'drawable-mdpi/node_modules_foo_node_modules_bar_asset.png'
)
)
).toBe(true);
} else {
expect(
fs.existsSync(path.join(dist, 'assets/node_modules/foo/asset.png'))
).toBe(true);
expect(
fs.existsSync(path.join(dist, 'assets/node_modules/baz/asset.png'))
).toBe(true);
expect(
fs.existsSync(
path.join(
dist,
'assets/node_modules/foo/node_modules/bar/asset.png'
)
)
).toBe(true);
}
}
}

test(`bundle ${platform} project`, () => {
const bundlePath = bundleForPlatform(PROJECT_FIXTURE, platform);
expect(fs.existsSync(bundlePath)).toBeTruthy();
assertAssets(bundlePath);
});

if (testRamBundle) {
test(`bundle ${platform} project as RAM bundle`, () => {
const bundlePath = bundleForPlatform(PROJECT_FIXTURE, platform, {
ramBundle: true,
});
expect(fs.existsSync(bundlePath)).toBeTruthy();
if (platform === 'android') {
expect(
fs.existsSync(
path.join(path.dirname(bundlePath), 'js-modules/UNBUNDLE')
)
).toBeTruthy();
}
assertAssets(bundlePath);
});
}
}
3 changes: 3 additions & 0 deletions fixtures/react_native_with_haul_0_60x/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,14 @@ import {
ReloadInstructions,
} from 'react-native/Libraries/NewAppScreen';

import Assets from 'foo';

const App = () => {
return (
<Fragment>
<StatusBar barStyle="dark-content" />
<SafeAreaView>
<Assets />
<ScrollView
contentInsetAdjustmentBehavior="automatic"
style={styles.scrollView}>
Expand Down
Binary file added fixtures/react_native_with_haul_0_60x/asset.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
8 changes: 8 additions & 0 deletions fixtures/react_native_with_haul_0_60x/bar/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import React from 'react';
import { Image } from 'react-native';

const Asset = () => (
<Image source={require('./asset.png')} />
);

export default Asset;
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
14 changes: 14 additions & 0 deletions fixtures/react_native_with_haul_0_60x/foo/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import React from 'react';
import { Image } from 'react-native';
import Baz from 'baz';
import Bar from 'bar';

export default () => (
<>
<Image source={require('./asset.png')} />
<Bar />
<Baz />
</>
);


36 changes: 36 additions & 0 deletions fixtures/react_native_with_haul_0_60x/haul.config.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,45 @@
import { withPolyfills, makeConfig } from "../../packages/haul-preset-0.60";
import path from 'path';

export default makeConfig({
bundles: {
index: {
entry: withPolyfills('./index.js'),
transform({ config }) {
config.resolve = {
...config.resolve,
modules: [
...(config.resolve.modules || []),
path.join(__dirname, './node_modules'),
path.join(__dirname, '../node_modules'),
path.join(__dirname, '../node_modules/foo/node_modules'),
]
};
config.module.rules.push(
{
test: /\.[jt]sx?$/,
include: [
path.join(__dirname, '../node_modules/foo'),
path.join(__dirname, '../node_modules/foo/node_modules/bar'),
path.join(__dirname, '../node_modules/baz')
],
use: [
{
loader: 'babel-loader',
options: {
extends: require.resolve('./babel.config.js'),
plugins: [
require.resolve(
'../../packages/haul-core/build/utils/fixRequireIssues'
),
],
cacheDirectory: false,
},
},
],
},
)
}
},
},
});
3 changes: 2 additions & 1 deletion fixtures/react_native_with_haul_0_60x/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
"scripts": {
"start": "node ../../packages/haul-cli/bin/haul.js start",
"test": "jest",
"haul": "node ../../packages/haul-cli/bin/haul.js"
"haul": "node ../../packages/haul-cli/bin/haul.js",
"postinstall": "rm -rf ../node_modules/foo ../node_modules/baz && mkdir -p ../node_modules/baz ../node_modules/foo/node_modules/bar && cp -R foo ../node_modules && cp -r bar/* ../node_modules/baz && cp -R bar ../node_modules/foo/node_modules"
thymikee marked this conversation as resolved.
Show resolved Hide resolved
},
"dependencies": {
"react": "16.8.6",
Expand Down
19 changes: 15 additions & 4 deletions packages/haul-core/src/webpack/loaders/assetLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,27 @@ async function assetLoader(this: any) {
const pathSepPattern = new RegExp(`\\${path.sep}`, 'g');
const filePath: string = this.resourcePath;
const dirname = path.dirname(filePath);
const url = path.relative(config.root, dirname);
// Relative path to root without any ../ due to https://github.com/callstack/haul/issues/474
// Assets from from outside of root, should still be placed inside bundle output directory.
// Example:
// filePath = monorepo/node_modules/my-module/image.png
// dirname = monorepo/node_modules/my-module
// root = monorepo/packages/my-app/
// url = ../../node_modules/my-module (original)
// So when we calculate destination for the asset for iOS ('assets' + url + filename),
// it will end up outside of `assets` directory, so we have to make sure it's:
// url = node_modules/my-module (tweaked)
const url = path
.relative(config.root, dirname)
.replace(new RegExp(`^[\\.\\${path.sep}]+`), '');
const type = path.extname(filePath).replace(/^\./, '');
const assets = path.join('assets', config.bundle ? '' : config.platform);
const suffix = `(@\\d+(\\.\\d+)?x)?(\\.(${config.platform}|native))?\\.${type}$`;
const filename = path.basename(filePath).replace(new RegExp(suffix), '');
const normalizedUrl = url.replace(new RegExp(`^[\\.\\${path.sep}]+`), '');
const normalizedName =
normalizedUrl.length === 0
url.length === 0
? filename
: `${normalizedUrl.replace(pathSepPattern, '_')}_${filename}`;
: `${url.replace(pathSepPattern, '_')}_${filename}`;
const longName = `${normalizedName
.toLowerCase()
.replace(/[^a-z0-9_]/g, '')}.${type}`;
Expand Down