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

Added @internal Js comments to internal API functions #2484

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@
"test:ci": "yarn check --integrity && npm run prettier:check && npm run lint -- --no-cache && npm run check && npm run testonly:cover && npm run check:ts && npm run check:spelling && npm run build",
"testonly": "mocha --full-trace src/**/__tests__/**/*-test.js",
"testonly:cover": "nyc npm run testonly",
"lint": "eslint --rulesdir './resources/eslint-rules' --rule 'no-dir-import: error' --cache --ext .js,.ts src resources",
"prelint": "node resources/generate-exported",
"lint": "eslint --rulesdir './resources/eslint-rules' --rule 'no-dir-import: error' --cache --ext .js,.ts src resources && eslint --rulesdir ./resources/eslint-rules/ --rule 'internal-func: 1' src --ignore-pattern 'src/jsutils/*' --ignore-pattern 'src/polyfills/*' --ignore-pattern 'src/**/__tests__/**' --ignore-pattern 'src/__tests__/*'",
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@IvanGoncharov, I've ignored all the files under __tests__ by ignoring them in the when we invoke eslint itself ( using --ignore-pattern ). But the command itself turned out to be larger in length... Should i break the lint commands in two parts or does this work?

"postlint": "rm resources/babel-plugins/map.json",
"benchmark": "node --noconcurrent_sweeping --expose-gc --predictable ./resources/benchmark.js",
"prettier": "prettier --ignore-path .gitignore --write --list-different \"**/*.{js,ts,md,json,yml}\"",
"prettier:check": "prettier --ignore-path .gitignore --check \"**/*.{js,ts,md,json,yml}\"",
Expand Down
117 changes: 117 additions & 0 deletions resources/babel-plugins/extract-exports.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
// @flow strict

'use strict';

// @noflow

const path = require('path');
const fs = require('fs');

const { HashMap } = require('./helper');

// Create a new Hashmap to store file names and declarations
const mp = new HashMap();

// All the rules that are required by plugin
const ExportNamedDeclaration = {
enter(babelPath, state) {
if (babelPath.node.declaration) {
return handleDeclaration(babelPath, state);
}

if (babelPath.node.specifiers) {
return handleNodeSpecifiers(
babelPath.node.specifiers,
state,
babelPath.node.source ? babelPath.node.source.value : undefined,
);
}
},
};

const ExportAllDeclaration = {
enter(babelPath, state) {
mp.add(
'*',
state,
babelPath.node.source ? babelPath.node.source.value : undefined,
);
},
};

module.exports = function() {
return {
visitor: {
ExportNamedDeclaration,
ExportAllDeclaration,
ExportDefaultDeclaration: ExportNamedDeclaration,
Program: {
exit() {
return writeToJSON();
},
},
},
};
};

// Helper functions for the rules
function handleDeclaration(babelPath, state) {
switch (babelPath.node.declaration.type) {
case 'VariableDeclaration':
return handleVariableDeclarations(
babelPath.node.declaration,
state,
babelPath.node.source ? babelPath.node.source.value : undefined,
);
case 'FunctionDeclaration':
case 'ClassDeclaration':
return handleFunctionDeclarations(
babelPath.node.declaration,
state,
babelPath.node.source ? babelPath.node.source.value : undefined,
);
}
}

function handleNodeSpecifiers(specifiers, state, source) {
return specifiers.forEach(specifier => {
switch (specifier.type) {
case 'ExportSpecifier':
mp.add(specifier.local.name, state, source);
break;
case 'ExportNamespaceSpecifier':
mp.add('*', state, source);
break;
}
});
}

function handleVariableDeclarations(variableDeclaration, state, source) {
variableDeclaration.declarations.forEach(declaration =>
mp.add(declaration.id.name, state, source),
);
}

function handleFunctionDeclarations(declaration, state, source) {
return mp.add(declaration.id.name, state, source);
}

// To write final result to JSON file
function writeToJSON() {
if (!fs.existsSync(path.join(__dirname, '/map.json'))) {
fs.writeFileSync(path.join(__dirname, '/map.json'), JSON.stringify({}));
}
const exportedValues = require(path.join(__dirname, '/map.json'));
for (const key of mp.keys()) {
exportedValues[key] = exportedValues[key] || [];

exportedValues[key] = exportedValues[key].concat(Array.from(mp.get(key)));

exportedValues[key] = Array.from(new Set(exportedValues[key]));
}

fs.writeFileSync(
path.join(__dirname, '/map.json'),
JSON.stringify(exportedValues),
);
}
33 changes: 33 additions & 0 deletions resources/babel-plugins/helper/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// @flow strict

'use strict';
const path = require('path');

class HashMap {
constructor() {
this._map = new Map();
}

add(value, state, source) {
let filepath = path.resolve(state.file.opts.filename);
if (source) {
const pathArray = state.file.opts.filename.split('/');
const directoryPath = pathArray.slice(0, pathArray.length - 1).join('/');
filepath = require.resolve(path.resolve(directoryPath, source));
}
if (!this._map.has(filepath)) {
this._map.set(filepath, new Set());
}
this._map.get(filepath).add(value);
}

get(key) {
return this._map.get(key);
}

keys() {
return this._map.keys();
}
}

module.exports = { HashMap };
49 changes: 49 additions & 0 deletions resources/eslint-rules/internal-func.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// @flow strict

// @noflow

'use strict';
const path = require('path');

const listOfExports = require(path.join(
process.cwd(),
'/resources/babel-plugins/',
'map.json',
));
module.exports = {
create(context) {
function isExportedLocallyOnly(name) {
if (!listOfExports[context.getFilename()]) {
return true;
}
return !listOfExports[context.getFilename()].find(
value => value === name || value === '*',
);
}

const source = context.getSourceCode();
/**
*
*/
return {
'ExportNamedDeclaration > :matches(FunctionDeclaration,ClassDeclaration)'(
node,
) {
if (isExportedLocallyOnly(node.id.name)) {
if (!source.getJSDocComment(node)) {
return context.report({
node,
message: 'Please enter JSDoC internal functions using @internal',
});
}
if (!source.getJSDocComment(node).value.includes('@internal')) {
context.report({
node,
message: 'Please annotate internal functions using @internal',
});
}
}
},
};
},
};
26 changes: 26 additions & 0 deletions resources/generate-exported.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// @noflow

'use strict';

const babel = require('@babel/core');
const flowTypesPlugin = require('@babel/plugin-transform-flow-strip-types');

const extractExportPlugin = require('./babel-plugins/extract-exports');

const directoriesToScan = [
'/src',
'/src/error',
'/src/type',
'/src/language',
'/src/validation',
'/src/utilities',
'/src/execution',
'/src/subscription',
];

directoriesToScan.forEach(path =>
babel.transformFileSync(process.cwd() + path + '/index.js', {
babelrc: false,
plugins: [flowTypesPlugin, extractExportPlugin],
}),
);
2 changes: 2 additions & 0 deletions src/language/ast.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { type TokenKindEnum } from './tokenKind';
/**
* Contains a range of UTF-8 character offsets and token references that
* identify the region of the source from which the AST derived.
* @internal
*/
export class Location {
/**
Expand Down Expand Up @@ -52,6 +53,7 @@ defineToJSON(Location, function() {
/**
* Represents a range of characters represented by a lexical token
* within a Source.
* @internal
*/
export class Token {
/**
Expand Down
4 changes: 4 additions & 0 deletions src/validation/ValidationContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ type VariableUsage = {|
* An instance of this class is passed as the "this" context to all validators,
* allowing access to commonly useful contextual information from within a
* validation rule.
* @internal
*/
export class ASTValidationContext {
_ast: DocumentNode;
Expand Down Expand Up @@ -133,6 +134,9 @@ export class ASTValidationContext {

export type ASTValidationRule = ASTValidationContext => ASTVisitor;

/**
* @internal
*/
export class SDLValidationContext extends ASTValidationContext {
_schema: ?GraphQLSchema;

Expand Down