Skip to content

Commit

Permalink
Restore InspectorProxy tests - fix Windows by Babel-ignoring `node_mo…
Browse files Browse the repository at this point in the history
…dules` consistently (#41526)

Summary:
Pull Request resolved: #41526

CI failures in Windows JS tests recently (#41463) were caused by the triggering of Babel registration during tests, due to an import of `packages/dev-middleware` (index), breaking subsequent transformation of other tests.

## Root cause
Example of a problematic import:
https://github.com/facebook/react-native/blob/a5d8ea4579c630af1e4e0fe1d99ad9dc0915df86/packages/dev-middleware/src/__tests__/ServerUtils.js#L15

..which triggers a Babel registration:
https://github.com/facebook/react-native/blob/a5d8ea4579c630af1e4e0fe1d99ad9dc0915df86/packages/dev-middleware/src/index.js#L16-L18

That registration behaves differently on Windows due to the `ignore: [/\/node_modules\/\]`, which doesn't match against Windows path separators - Babel matches against system separators.

In particular, this changed whether `node_modules/flow-parser` was transformed when loading the RN Babel transformer. Transforming this file causes a `console.warn` from Babel due to its size:
> [BABEL] Note: The code generator has deoptimised the styling of /Users/robhogan/workspace/react-native/node_modules/flow-parser/flow_parser.js as it exceeds the max of 500KB.

This throws due to our setup:
https://github.com/facebook/react-native/blob/a5d8ea4579c630af1e4e0fe1d99ad9dc0915df86/packages/react-native/jest/local-setup.js#L27

This all manifests as the first test following a Babel registration (within the same Jest worker) that requires the RN Babel transformer throwing during script transformation.

## This change
This is the minimally disruptive change that makes Babel registration behaviour consistent between Windows and other platforms. The more durable solution here would be *not* to rely on any Babel registration for Jest, which has its own `ScriptTransformer` mechanism for running code from source. Given the fragile way our internal+OSS Babel set up hangs together that's a higher-risk change, so I'll follow up separately.

Changelog: [Internal]

Reviewed By: huntie

Differential Revision: D51424802

fbshipit-source-id: 8b733c0c159ee84690aef04abced682d126c6d27
  • Loading branch information
robhogan authored and facebook-github-bot committed Nov 17, 2023
1 parent 5e7be7e commit 59a2282
Show file tree
Hide file tree
Showing 5 changed files with 5 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@ beforeAll(() => {
jest.resetModules();
});

// TODO T169943794
xdescribe.each(['HTTP', 'HTTPS'])(
describe.each(['HTTP', 'HTTPS'])(
'inspector proxy CDP rewriting hacks over %s',
protocol => {
const serverRef = withServerForEachTest({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ jest.useRealTimers();

jest.setTimeout(10000);

// TODO T169943794
xdescribe.each(['HTTP', 'HTTPS'])(
describe.each(['HTTP', 'HTTPS'])(
'inspector proxy CDP transport over %s',
protocol => {
const serverRef = withServerForEachTest({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ const PAGES_POLLING_DELAY = 1000;

jest.useFakeTimers();

// TODO T169943794
xdescribe('inspector proxy HTTP API', () => {
describe('inspector proxy HTTP API', () => {
const serverRef = withServerForEachTest({
logger: undefined,
projectRoot: '',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ jest.useRealTimers();

jest.setTimeout(10000);

// TODO T169943794
xdescribe('inspector proxy React Native reloads', () => {
describe('inspector proxy React Native reloads', () => {
const serverRef = withServerForEachTest({
logger: undefined,
projectRoot: '',
Expand Down
2 changes: 1 addition & 1 deletion scripts/build/babel-register.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ function registerPackage(packageName /*: string */) {
const registerConfig = {
...getBabelConfig(packageName),
root: packageDir,
ignore: [/\/node_modules\//],
ignore: [/[/\\]node_modules[/\\]/],
};

require('@babel/register')(registerConfig);
Expand Down

0 comments on commit 59a2282

Please sign in to comment.