Skip to content

Commit

Permalink
[docgen] Refactor code to use require() instead of JSON.parse(fs.read…
Browse files Browse the repository at this point in the history 
…FileSync())

While working through #37929 I noticed that the tests in the `docgen` package
read fixture data manually by chaining `JSON.parse()` and `fs.readFileSync()`,
often at times separated by a number of lines.

In this patch we're replacing those with the more natural `require()` statement
which removes noise from the test modules and hopefully will also remove a point
of confusion for those coming in to modify it. We shouldn't have to ask, "why is
this code doing something that looks normal but is doing it in a notably different
manner?"

After reviewing the history of the work, initially introduced in #13329, I could
find no explanation for the approach and found no discussion about it in the PR.
  • Loading branch information
@dmsnell
dmsnell committed Jan 21, 2022
1 parent 84b8887 commit 695cb91
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 504 deletions.
197 changes: 44 additions & 153 deletions packages/docgen/test/get-export-entries.js
View file
Original file line number Diff line number Diff line change
@@ -1,24 +1,12 @@
/**
* External dependencies
*/
const fs = require( 'fs' );
const path = require( 'path' );

/**
* Internal dependencies
*/
const getExportEntries = require( '../lib/get-export-entries' );

describe( 'Export entries', () => {
it( 'default class (anonymous)', () => {
const token = fs.readFileSync(
path.join(
__dirname,
'./fixtures/default-class-anonymous/exports.json'
),
'utf-8'
);
const name = getExportEntries( JSON.parse( token ) );
const token = require( './fixtures/default-class-anonymous/exports.json' );
const name = getExportEntries( token );
expect( name ).toHaveLength( 1 );
expect( name[ 0 ] ).toEqual( {
localName: '*default*',
Expand All @@ -30,14 +18,8 @@ describe( 'Export entries', () => {
} );

it( 'default class (named)', () => {
const token = fs.readFileSync(
path.join(
__dirname,
'./fixtures/default-class-named/exports.json'
),
'utf-8'
);
const name = getExportEntries( JSON.parse( token ) );
const token = require( './fixtures/default-class-named/exports.json' );
const name = getExportEntries( token );
expect( name ).toHaveLength( 1 );
expect( name[ 0 ] ).toEqual( {
localName: 'ClassDeclaration',
Expand All @@ -49,14 +31,8 @@ describe( 'Export entries', () => {
} );

it( 'default function (anonymous)', () => {
const token = fs.readFileSync(
path.join(
__dirname,
'./fixtures/default-function-anonymous/exports.json'
),
'utf-8'
);
const name = getExportEntries( JSON.parse( token ) );
const token = require( './fixtures/default-function-anonymous/exports.json' );
const name = getExportEntries( token );
expect( name ).toHaveLength( 1 );
expect( name[ 0 ] ).toEqual( {
localName: '*default*',
Expand All @@ -68,14 +44,8 @@ describe( 'Export entries', () => {
} );

it( 'default function (named)', () => {
const token = fs.readFileSync(
path.join(
__dirname,
'./fixtures/default-function-named/exports.json'
),
'utf-8'
);
const name = getExportEntries( JSON.parse( token ) );
const token = require( './fixtures/default-function-named/exports.json' );
const name = getExportEntries( token );
expect( name ).toHaveLength( 1 );
expect( name[ 0 ] ).toEqual( {
localName: 'myDeclaration',
Expand All @@ -87,14 +57,8 @@ describe( 'Export entries', () => {
} );

it( 'default identifier', () => {
const token = fs.readFileSync(
path.join(
__dirname,
'./fixtures/default-identifier/exports.json'
),
'utf-8'
);
const name = getExportEntries( JSON.parse( token ) );
const token = require( './fixtures/default-identifier/exports.json' );
const name = getExportEntries( token );
expect( name ).toHaveLength( 1 );
expect( name[ 0 ] ).toEqual( {
localName: 'ClassDeclaration',
Expand All @@ -106,14 +70,8 @@ describe( 'Export entries', () => {
} );

it( 'default import (named)', () => {
const token = fs.readFileSync(
path.join(
__dirname,
'./fixtures/default-import-named/exports.json'
),
'utf-8'
);
const name = getExportEntries( JSON.parse( token ) );
const token = require( './fixtures/default-import-named/exports.json' );
const name = getExportEntries( token );
expect( name ).toHaveLength( 1 );
expect( name[ 0 ] ).toEqual( {
localName: 'fnDeclaration',
Expand All @@ -125,14 +83,8 @@ describe( 'Export entries', () => {
} );

it( 'default import (default)', () => {
const token = fs.readFileSync(
path.join(
__dirname,
'./fixtures/default-import-default/exports.json'
),
'utf-8'
);
const name = getExportEntries( JSON.parse( token ) );
const token = require( './fixtures/default-import-default/exports.json' );
const name = getExportEntries( token );
expect( name ).toHaveLength( 1 );
expect( name[ 0 ] ).toEqual( {
localName: 'fnDeclaration',
Expand All @@ -144,14 +96,8 @@ describe( 'Export entries', () => {
} );

it( 'default named export', () => {
const tokens = fs.readFileSync(
path.join(
__dirname,
'./fixtures/default-named-export/exports.json'
),
'utf-8'
);
const namedExport = getExportEntries( JSON.parse( tokens )[ 0 ] );
const tokens = require( './fixtures/default-named-export/exports.json' );
const namedExport = getExportEntries( tokens[ 0 ] );
expect( namedExport ).toHaveLength( 1 );
expect( namedExport[ 0 ] ).toEqual( {
localName: 'functionDeclaration',
Expand All @@ -160,7 +106,7 @@ describe( 'Export entries', () => {
lineStart: 4,
lineEnd: 4,
} );
const defaultExport = getExportEntries( JSON.parse( tokens )[ 1 ] );
const defaultExport = getExportEntries( tokens[ 1 ] );
expect( defaultExport ).toHaveLength( 1 );
expect( defaultExport[ 0 ] ).toEqual( {
localName: 'functionDeclaration',
Expand All @@ -172,11 +118,8 @@ describe( 'Export entries', () => {
} );

it( 'default variable', () => {
const token = fs.readFileSync(
path.join( __dirname, './fixtures/default-variable/exports.json' ),
'utf-8'
);
const name = getExportEntries( JSON.parse( token ) );
const token = require( './fixtures/default-variable/exports.json' );
const name = getExportEntries( token );
expect( name ).toHaveLength( 1 );
expect( name[ 0 ] ).toEqual( {
localName: '*default*',
Expand All @@ -188,11 +131,8 @@ describe( 'Export entries', () => {
} );

it( 'named class', () => {
const token = fs.readFileSync(
path.join( __dirname, './fixtures/named-class/exports.json' ),
'utf-8'
);
const name = getExportEntries( JSON.parse( token ) );
const token = require( './fixtures/named-class/exports.json' );
const name = getExportEntries( token );
expect( name ).toHaveLength( 1 );
expect( name[ 0 ] ).toEqual( {
localName: 'MyDeclaration',
Expand All @@ -204,11 +144,8 @@ describe( 'Export entries', () => {
} );

it( 'named default', () => {
const token = fs.readFileSync(
path.join( __dirname, './fixtures/named-default/exports.json' ),
'utf-8'
);
const name = getExportEntries( JSON.parse( token ) );
const token = require( './fixtures/named-default/exports.json' );
const name = getExportEntries( token );
expect( name ).toHaveLength( 1 );
expect( name[ 0 ] ).toEqual( {
localName: 'default',
Expand All @@ -220,14 +157,8 @@ describe( 'Export entries', () => {
} );

it( 'named default (exported)', () => {
const token = fs.readFileSync(
path.join(
__dirname,
'./fixtures/named-default-exported/exports.json'
),
'utf-8'
);
const name = getExportEntries( JSON.parse( token ) );
const token = require( './fixtures/named-default-exported/exports.json' );
const name = getExportEntries( token );
expect( name ).toHaveLength( 1 );
expect( name[ 0 ] ).toEqual( {
localName: 'default',
Expand All @@ -239,11 +170,8 @@ describe( 'Export entries', () => {
} );

it( 'named function', () => {
const token = fs.readFileSync(
path.join( __dirname, './fixtures/named-function/exports.json' ),
'utf-8'
);
const name = getExportEntries( JSON.parse( token ) );
const token = require( './fixtures/named-function/exports.json' );
const name = getExportEntries( token );
expect( name ).toHaveLength( 1 );
expect( name[ 0 ] ).toEqual( {
localName: 'myDeclaration',
Expand All @@ -255,11 +183,8 @@ describe( 'Export entries', () => {
} );

it( 'named identifier', () => {
const token = fs.readFileSync(
path.join( __dirname, './fixtures/named-identifier/exports.json' ),
'utf-8'
);
const name = getExportEntries( JSON.parse( token ) );
const token = require( './fixtures/named-identifier/exports.json' );
const name = getExportEntries( token );
expect( name ).toHaveLength( 1 );
expect( name[ 0 ] ).toEqual( {
localName: 'myDeclaration',
Expand All @@ -268,14 +193,8 @@ describe( 'Export entries', () => {
lineStart: 6,
lineEnd: 6,
} );
const tokenObject = fs.readFileSync(
path.join(
__dirname,
'./fixtures/named-identifier-destructuring/exports.json'
),
'utf-8'
);
const nameObject = getExportEntries( JSON.parse( tokenObject ) );
const tokenObject = require( './fixtures/named-identifier-destructuring/exports.json' );
const nameObject = getExportEntries( tokenObject );
expect( nameObject ).toHaveLength( 1 );
expect( nameObject[ 0 ] ).toEqual( {
localName: 'someDeclaration',
Expand All @@ -287,11 +206,8 @@ describe( 'Export entries', () => {
} );

it( 'named identifiers', () => {
const token = fs.readFileSync(
path.join( __dirname, './fixtures/named-identifiers/exports.json' ),
'utf-8'
);
const name = getExportEntries( JSON.parse( token ) );
const token = require( './fixtures/named-identifiers/exports.json' );
const name = getExportEntries( token );
expect( name ).toHaveLength( 3 );
expect( name[ 0 ] ).toEqual( {
localName: 'functionDeclaration',
Expand All @@ -314,16 +230,8 @@ describe( 'Export entries', () => {
lineStart: 16,
lineEnd: 16,
} );
const tokenIdentifiersAndInline = fs.readFileSync(
path.join(
__dirname,
'./fixtures/named-identifiers-and-inline/exports.json'
),
'utf-8'
);
const nameInline0 = getExportEntries(
JSON.parse( tokenIdentifiersAndInline )[ 0 ]
);
const tokenIdentifiersAndInline = require( './fixtures/named-identifiers-and-inline/exports.json' );
const nameInline0 = getExportEntries( tokenIdentifiersAndInline[ 0 ] );
expect( nameInline0 ).toHaveLength( 2 );
expect( nameInline0[ 0 ] ).toEqual( {
localName: 'functionDeclaration',
Expand All @@ -339,9 +247,7 @@ describe( 'Export entries', () => {
lineStart: 11,
lineEnd: 11,
} );
const nameInline1 = getExportEntries(
JSON.parse( tokenIdentifiersAndInline )[ 1 ]
);
const nameInline1 = getExportEntries( tokenIdentifiersAndInline[ 1 ] );
expect( nameInline1 ).toHaveLength( 1 );
expect( nameInline1[ 0 ] ).toEqual( {
localName: 'variableDeclaration',
Expand All @@ -353,14 +259,8 @@ describe( 'Export entries', () => {
} );

it( 'named import namespace', () => {
const token = fs.readFileSync(
path.join(
__dirname,
'./fixtures/named-import-namespace/exports.json'
),
'utf-8'
);
const name = getExportEntries( JSON.parse( token ) );
const token = require( './fixtures/named-import-namespace/exports.json' );
const name = getExportEntries( token );
expect( name ).toHaveLength( 1 );
expect( name[ 0 ] ).toEqual( {
localName: 'variables',
Expand All @@ -372,11 +272,8 @@ describe( 'Export entries', () => {
} );

it( 'named variable', () => {
const token = fs.readFileSync(
path.join( __dirname, './fixtures/named-variable/exports.json' ),
'utf-8'
);
const name = getExportEntries( JSON.parse( token ) );
const token = require( './fixtures/named-variable/exports.json' );
const name = getExportEntries( token );
expect( name ).toHaveLength( 1 );
expect( name[ 0 ] ).toEqual( {
localName: 'myDeclaration',
Expand All @@ -388,11 +285,8 @@ describe( 'Export entries', () => {
} );

it( 'named variables', () => {
const token = fs.readFileSync(
path.join( __dirname, './fixtures/named-variables/exports.json' ),
'utf-8'
);
const name = getExportEntries( JSON.parse( token ) );
const token = require( './fixtures/named-variables/exports.json' );
const name = getExportEntries( token );
expect( name ).toHaveLength( 2 );
expect( name[ 0 ] ).toEqual( {
localName: 'firstDeclaration',
Expand All @@ -411,11 +305,8 @@ describe( 'Export entries', () => {
} );

it( 'namespace (*)', () => {
const token = fs.readFileSync(
path.join( __dirname, './fixtures/namespace/exports.json' ),
'utf-8'
);
const name = getExportEntries( JSON.parse( token ) );
const token = require( './fixtures/namespace/exports.json' );
const name = getExportEntries( token );
expect( name ).toHaveLength( 1 );
expect( name[ 0 ] ).toEqual( {
localName: '*',
Expand Down
Loading

0 comments on commit 695cb91

Please sign in to comment.