Skip to content

Commit

Permalink
Revert D50939417: Add function names to module factories in DEV
Browse files Browse the repository at this point in the history
Differential Revision:
D50939417

Original commit changeset: 3ad1226d0006

Original Phabricator Diff: D50939417

fbshipit-source-id: 3dd0ef873db97c95fd316f7f7f034ca578478d02
  • Loading branch information
robhogan authored and facebook-github-bot committed Nov 8, 2023
1 parent 1e1462f commit 92340b3
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 71 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ Object {
`;

exports[`transforms an es module with asyncToGenerator 1`] = `
"__d(function local_file_js__module_factory__(global, _$$_REQUIRE, _$$_IMPORT_DEFAULT, _$$_IMPORT_ALL, module, exports, _dependencyMap) {
"__d(function (global, _$$_REQUIRE, _$$_IMPORT_DEFAULT, _$$_IMPORT_ALL, module, exports, _dependencyMap) {
var _interopRequireDefault = _$$_REQUIRE(_dependencyMap[0], \\"@babel/runtime/helpers/interopRequireDefault\\");
Object.defineProperty(exports, \\"__esModule\\", {
value: true
Expand Down Expand Up @@ -268,7 +268,7 @@ Object {
`;

exports[`transforms async generators 1`] = `
"__d(function local_file_js__module_factory__(global, _$$_REQUIRE, _$$_IMPORT_DEFAULT, _$$_IMPORT_ALL, module, exports, _dependencyMap) {
"__d(function (global, _$$_REQUIRE, _$$_IMPORT_DEFAULT, _$$_IMPORT_ALL, module, exports, _dependencyMap) {
var _interopRequireDefault = _$$_REQUIRE(_dependencyMap[0], \\"@babel/runtime/helpers/interopRequireDefault\\");
Object.defineProperty(exports, \\"__esModule\\", {
value: true
Expand Down
47 changes: 9 additions & 38 deletions packages/metro-transform-worker/src/__tests__/index-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,9 @@ const babelTransformerPath = require.resolve(
const transformerContents = (() =>
require('fs').readFileSync(babelTransformerPath))();

function getExpectedHeaderDev(expectedInferredFactoryName: string) {
return `__d(function ${expectedInferredFactoryName}__module_factory__(global, _$$_REQUIRE, _$$_IMPORT_DEFAULT, _$$_IMPORT_ALL, module, exports, _dependencyMap) {`;
}

function getExpectedHeaderProd() {
return '__d(function (g, r, i, a, m, e, d) {';
}
const HEADER_DEV =
'__d(function (global, _$$_REQUIRE, _$$_IMPORT_DEFAULT, _$$_IMPORT_ALL, module, exports, _dependencyMap) {';
const HEADER_PROD = '__d(function (g, r, i, a, m, e, d) {';

let fs: FSType;
let Transformer: TransformerType;
Expand Down Expand Up @@ -134,9 +130,7 @@ it('transforms a simple module', async () => {

expect(result.output[0].type).toBe('js/module');
expect(result.output[0].data.code).toBe(
[getExpectedHeaderDev('local_file_js'), ' arbitrary(code);', '});'].join(
'\n',
),
[HEADER_DEV, ' arbitrary(code);', '});'].join('\n'),
);
expect(result.output[0].data.map).toMatchSnapshot();
expect(result.output[0].data.functionMap).toMatchSnapshot();
Expand All @@ -163,7 +157,7 @@ it('transforms a module with dependencies', async () => {
expect(result.output[0].type).toBe('js/module');
expect(result.output[0].data.code).toBe(
[
getExpectedHeaderDev('local_file_js'),
HEADER_DEV,
' "use strict";',
'',
' var _interopRequireDefault = _$$_REQUIRE(_dependencyMap[0], "@babel/runtime/helpers/interopRequireDefault");',
Expand Down Expand Up @@ -252,7 +246,7 @@ it('transforms import/export syntax when experimental flag is on', async () => {
expect(result.output[0].type).toBe('js/module');
expect(result.output[0].data.code).toBe(
[
getExpectedHeaderDev('local_file_js'),
HEADER_DEV,
' "use strict";',
'',
' var c = _$$_IMPORT_DEFAULT(_dependencyMap[0], "./c");',
Expand Down Expand Up @@ -282,11 +276,7 @@ it('does not add "use strict" on non-modules', async () => {

expect(result.output[0].type).toBe('js/module');
expect(result.output[0].data.code).toBe(
[
getExpectedHeaderDev('node_modules_local_file_js'),
' module.exports = {};',
'});',
].join('\n'),
[HEADER_DEV, ' module.exports = {};', '});'].join('\n'),
);
});

Expand Down Expand Up @@ -345,7 +335,7 @@ it('supports dynamic dependencies from within `node_modules`', async () => {
).output[0].data.code,
).toBe(
[
getExpectedHeaderDev('node_modules_foo_bar_js'),
HEADER_DEV,
' (function (line) {',
" throw new Error('Dynamic require defined at line ' + line + '; not supported by Metro');",
' })(1);',
Expand All @@ -365,7 +355,7 @@ it('minifies the code correctly', async () => {
{...baseTransformOptions, minify: true},
)
).output[0].data.code,
).toBe([getExpectedHeaderProd(), ' minified(code);', '});'].join('\n'));
).toBe([HEADER_PROD, ' minified(code);', '});'].join('\n'));
});

it('minifies a JSON file', async () => {
Expand Down Expand Up @@ -572,22 +562,3 @@ it('allows outputting comments when `minify: true`', async () => {
});"
`);
});

it('omits invalid characters from module factory name', async () => {
const result = await Transformer.transform(
baseConfig,
'/root',
'local/!@£% weird name.js',
Buffer.from('arbitrary(code)', 'utf8'),
baseTransformOptions,
);

expect(result.output[0].type).toBe('js/module');
expect(result.output[0].data.code).toBe(
[
getExpectedHeaderDev('local______weird_name_js'),
' arbitrary(code);',
'});',
].join('\n'),
);
});
15 changes: 5 additions & 10 deletions packages/metro-transform-worker/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -346,11 +346,6 @@ async function transformJS(
let dependencies;
let wrappedAst;

const minify =
options.minify &&
options.unstable_transformProfile !== 'hermes-canary' &&
options.unstable_transformProfile !== 'hermes-stable';

// If the module to transform is a script (meaning that is not part of the
// dependency graph and it code will just be prepended to the bundle modules),
// we need to wrap it differently than a commonJS module (also, scripts do
Expand Down Expand Up @@ -387,21 +382,21 @@ async function transformJS(
if (config.unstable_disableModuleWrapping === true) {
wrappedAst = ast;
} else {
let moduleFactoryName;
if (options.dev && !minify) {
moduleFactoryName = file.filename;
}
({ast: wrappedAst} = JsFileWrapping.wrapModule(
ast,
importDefault,
importAll,
dependencyMapName,
config.globalPrefix,
moduleFactoryName,
));
}
}

const minify =
options.minify &&
options.unstable_transformProfile !== 'hermes-canary' &&
options.unstable_transformProfile !== 'hermes-stable';

const reserved = [];
if (config.unstable_dependencyMapReservedName != null) {
reserved.push(config.unstable_dependencyMapReservedName);
Expand Down
23 changes: 2 additions & 21 deletions packages/metro/src/ModuleGraph/worker/JsFileWrapping.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ function wrapModule(
importAllName: string,
dependencyMapName: string,
globalPrefix: string,
moduleFactoryName?: string,
): {
ast: BabelNodeFile,
requireName: string,
Expand All @@ -42,11 +41,7 @@ function wrapModule(
importAllName,
dependencyMapName,
);
const factory = functionFromProgram(
fileAst.program,
params,
moduleFactoryName,
);
const factory = functionFromProgram(fileAst.program, params);
const def = t.callExpression(t.identifier(`${globalPrefix}__d`), [factory]);
const ast = t.file(t.program([t.expressionStatement(def)]));

Expand Down Expand Up @@ -81,26 +76,12 @@ function wrapJson(source: string, globalPrefix: string): string {
].join('\n');
}

const JS_INVALID_IDENT_RE = /[^a-zA-Z0-9$_]/g;

function functionFromProgram(
program: Program,
parameters: $ReadOnlyArray<string>,
moduleFactoryName?: string,
): FunctionExpression {
let identifier;
if (typeof moduleFactoryName === 'string' && moduleFactoryName !== '') {
// Keep the name readable so it shows up in profiler traces.
// Add an unlikely suffix to avoid collisions with the module code.
identifier = t.identifier(
`${moduleFactoryName.replace(
JS_INVALID_IDENT_RE,
'_',
)}__module_factory__`,
);
}
return t.functionExpression(
identifier,
undefined,
parameters.map(makeIdentifier),
t.blockStatement(program.body, program.directives),
);
Expand Down

0 comments on commit 92340b3

Please sign in to comment.