Skip to content

Commit

Permalink
[BREAKING] feat: make resolver preset follow Metro defaults closely (#…
Browse files Browse the repository at this point in the history
…525)

* fix: getResolveOptions implementation

* chore: add some additional descriptions

* refactor: don't use default initializer, improve JSDoc

* chore: make sure typedoc/jsdoc looks good

* chore: add descriptive changeset

* fix: mark options as optional

* fix: align behaviour of conditions by platform in tests

* refactor: add more tests to skip in metro-compat-test

* test: enable metro-compat-test on CI

* feat: use the same order of extensions as metro

* chore: update changeset
  • Loading branch information
jbroma committed Apr 5, 2024
1 parent 1114a53 commit a74930b
Show file tree
Hide file tree
Showing 5 changed files with 119 additions and 29 deletions.
15 changes: 15 additions & 0 deletions .changeset/kind-scissors-kick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
"@callstack/repack": major
---

BREAKING CHANGE:

`getResolveOptions` is now way more compatible with `metro-resolver` and `@react-native/metro-config`

1. `getResolveOptions` now accepts a second optional parameter called options with the following properties:
- `enablePackageExports` - defaults to `false`
- `preferNativePlatform` - defaults to `true`
2. Order of extensions was changed to match the order from `@react-native/metro-config`.
3. Resolution via Package Exports (`exports` field in package.json) is now optional and disabled by default.
It can now be enabled via `getResolveOptions` options parameter. This change was introduced to match `metro` defaults.
4. Default `conditionNames` are now: `['require', 'import', 'react-native']` and match `@react-native/metro-config` defaults.
19 changes: 17 additions & 2 deletions metro-compat-test/jest.setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,39 @@ const testsToSkip = {
'[nonstrict] should fall back to "main" field resolution when "exports" is an invalid subpath',
'[nonstrict] should fall back to "browser" spec resolution and log inaccessible import warning',
'[nonstrict] should fall back and log warning for an invalid "exports" target value',
'[nonstrict] should fall back to "browser" spec resolution and log inaccessible import warning',
'[nonstrict] should fall back to "browser" spec resolution and log inaccessible import warning',
// Assets are handled differently in webpack
'should resolve assets using "exports" field and calling `resolveAsset`',
// Resolving fails as expected but error messages are different
'should use most specific pattern base',
'should throw FailedToResolvePathError when no conditions are matched',
// Root shorthand - pending fix in metro
'should expand array of strings as subpath mapping (root shorthand)',
// Restricted imports - pending fix in metro
'should resolve subpath patterns in "exports" matching import specifier',
]),
};

const testsToSkipOnce = {
describe: new Set(),
test: new Set([
// sourceExts are expanded, platform-specific extensions are not
'without expanding `sourceExts`',
'without expanding platform-specific extensions',
]),
};

// alias it to test
Object.defineProperty(testsToSkip, 'it', testsToSkip.test);
Object.defineProperty(testsToSkipOnce, 'it', testsToSkipOnce.test);

// trap call & check if test should be skipped
const handler = {
apply(target, _, args) {
if (testsToSkip[target.name].has(args[0])) {
return target.skip(...args);
} else if (testsToSkipOnce[target.name].has(args[0])) {
testsToSkipOnce[target.name].delete(args[0]);
return target.skip(...args);
} else return target(...args);
},
};
Expand Down
9 changes: 6 additions & 3 deletions metro-compat-test/resolver/resolve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,12 @@ function transformContext(
} as InputFileMap,
options: {
// repack uses whole separate config per platform
conditionNames: platform
? context.unstable_conditionsByPlatform[platform]
: context.unstable_conditionNames,
conditionNames: Array.from(
new Set([
...context.unstable_conditionNames,
...(context.unstable_conditionsByPlatform[platform!] ?? []),
])
),
fallback: context.extraNodeModules
? { ...context.extraNodeModules }
: undefined,
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"build": "turbo run build",
"lint": "turbo run lint --filter=!metro-compat-test",
"typecheck": "turbo run typecheck --filter=!metro-compat-test",
"test": "turbo run test --filter=!metro-compat-test",
"test": "turbo run test",
"release": "pnpm build && pnpm lint && pnpm test && pnpm changeset publish",
"website:start": "turbo run docs && pnpm --filter website run start",
"website:build": "turbo run docs && pnpm --filter website run export",
Expand Down
103 changes: 80 additions & 23 deletions packages/repack/src/webpack/utils/getResolveOptions.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,32 @@
/**
* Get Webpack's resolve options to properly resolve JavaScript files
* that contain `<platform>` or `native` (eg `file.ios.js`) suffixes as well as `react-native` field
* in dependencies' `package.json`.
* {@link getResolveOptions} additional options.
*/
export interface ResolveOptions {
/**
* Whether to enable Package Exports support. Defaults to `false`.
*/
enablePackageExports?: boolean;
/**
* Whether to prefer native platform over generic platform. Defaults to `true`
*/
preferNativePlatform?: boolean;
}

/**
* Get Webpack's resolve options to properly resolve JavaScript files:
* - resolve platform extensions (e.g. `file.ios.js`)
* - resolve native extensions (e.g. `file.native.js`)
* - optionally use package exports (`exports` field in `package.json`) instead of
* main fields (e.g. `main` or `browser` or `react-native`)
*
* @param platform Target application platform.
* @param options Additional options that can modify resolution behaviour.
* @returns Webpack's resolve options.
*
* @category Webpack util
*
* @example Usage in Webpack config:
*
* ```ts
* import * as Repack from '@callstack/repack';
*
Expand All @@ -17,35 +35,74 @@
*
* return {
* resolve: {
* ...Repack.getResolveOptions(platform),
* ...Repack.getResolveOptions(platform, {
* enablePackageExports: false,
* preferNativePlatform: true
* }),
* },
* };
* };
* ```
*/
export function getResolveOptions(platform: string) {

export function getResolveOptions(platform: string, options?: ResolveOptions) {
const preferNativePlatform = options?.preferNativePlatform ?? true;
const enablePackageExports = options?.enablePackageExports ?? false;

let extensions = ['.js', '.jsx', '.json', '.ts', '.tsx'];

let conditionNames: string[];
let exportsFields: string[];

if (enablePackageExports) {
/**
* Match what React Native uses in @react-native/metro-config.
* Order of conditionNames doesn't matter.
* Order inside of target package.json's `exports` field matters.
*/
conditionNames = ['require', 'import', 'react-native'];
exportsFields = ['exports'];
} else {
conditionNames = [];
exportsFields = [];
extensions = extensions.flatMap((ext) => {
const platformExt = `.${platform}${ext}`;
const nativeExt = `.native${ext}`;

if (preferNativePlatform) {
return [platformExt, nativeExt, ext];
} else {
return [platformExt, ext];
}
});
}

/**
* Match what React Native uses in @react-native/metro-config.
* First entry takes precedence.
*
* Reference: https://github.com/facebook/react-native/blob/main/packages/metro-config/src/index.flow.js
*/
return {
/**
* Match what React Native packager supports.
* First entry takes precedence.
* Reference: Webpack's [configuration.resolve.mainFields](https://webpack.js.org/configuration/resolve/#resolvemainfields)
*/
mainFields: ['react-native', 'browser', 'main'],
/**
* Reference: Webpack's [configuration.resolve.aliasFields](https://webpack.js.org/configuration/resolve/#resolvealiasfields)
*/
aliasFields: ['react-native', 'browser', 'main'],
conditionNames: ['default', 'require'],
extensions: [
`.${platform}.ts`,
`.${platform}.js`,
`.${platform}.tsx`,
`.${platform}.jsx`,
'.native.ts',
'.native.js',
'.native.tsx',
'.native.jsx',
'.ts',
'.js',
'.tsx',
'.jsx',
'.json',
],
/**
* Reference: Webpack's [configuration.resolve.conditionNames](https://webpack.js.org/configuration/resolve/#resolveconditionnames)
*/
conditionNames: conditionNames,
/**
* Reference: Webpack's [configuration.resolve.exportsFields](https://webpack.js.org/configuration/resolve/#resolveexportsfields)
*/
exportsFields: exportsFields,
/**
* Reference: Webpack's [configuration.resolve.extensions](https://webpack.js.org/configuration/resolve/#resolveextensions)
*/
extensions: extensions,
};
}

0 comments on commit a74930b

Please sign in to comment.