Skip to content

Commit

Permalink
lint: support typescript (ampproject#37324)
Browse files Browse the repository at this point in the history
* lint: support typescript

* low hanging fruit

* Lint for rule-of-hooks

* for ts files, don't need all the jsdoc rules

* more low hanging fruit

* fix import assertions

* unused incorrect ordering

* rcebulko finds.

* remove sort-requires

* Fix newly found BS errors from TS Upgrade.

* jridgewell feedback
  • Loading branch information
samouri authored Jan 12, 2022
1 parent d0b2ed8 commit b2e9a06
Show file tree
Hide file tree
Showing 67 changed files with 699 additions and 315 deletions.
20 changes: 14 additions & 6 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ function getExperimentGlobals() {

module.exports = {
'root': true,
'parser': '@babel/eslint-parser',
'parser': '@typescript-eslint/parser',
'plugins': [
'chai-expect',
'import',
Expand All @@ -39,14 +39,13 @@ module.exports = {
'react',
'react-hooks',
'sort-destructure-keys',
'sort-requires',
'@typescript-eslint',
],
'env': {
'es6': true,
'browser': true,
},
'parserOptions': {
'ecmaVersion': 6,
'jsx': true,
'sourceType': 'module',
},
Expand Down Expand Up @@ -260,7 +259,8 @@ module.exports = {
'no-sequences': 2,
'no-throw-literal': 2,
'no-unused-expressions': 0,
'no-unused-vars': [
'no-unused-vars': 'off',
'@typescript-eslint/no-unused-vars': [
2,
{
'argsIgnorePattern': '^(var_args$|opt_|unused)',
Expand Down Expand Up @@ -341,9 +341,17 @@ module.exports = {
'ignoreDeclarationSort': true,
},
],
'sort-requires/sort-requires': 2,
},
'overrides': [
{
'files': ['**/*.ts'],
'rules': {
'require-jsdoc': 0,
'jsdoc/require-param': 0,
'jsdoc/require-param-type': 0,
'jsdoc/require-returns': 0,
},
},
{
'files': [
'test/**/*.js',
Expand Down Expand Up @@ -432,7 +440,7 @@ module.exports = {
'rules': {
'no-var': 0,
'no-undef': 0,
'no-unused-vars': 0,
'@typescript-eslint/no-unused-vars': 0,
'prefer-const': 0,
'require-jsdoc': 0,
'jsdoc/check-tag-names': 0,
Expand Down
2 changes: 1 addition & 1 deletion ads/vendors/csa.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ function orientationChangeHandler(global, containerDiv) {
const oldHeight = getStyle(containerDiv, 'height');
global.setTimeout(() => {
// Force DOM reflow and repaint.
// eslint-disable-next-line no-unused-vars
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const ignore = global.document.body./*OK*/ offsetHeight;
// Capture new height.
const newHeight = getStyle(containerDiv, 'height');
Expand Down
4 changes: 2 additions & 2 deletions build-system/common/update-session-issues/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
* ⚠️ Only use standard node modules.
* This file cannot depend on `npm install`.
*/
const https = require('https');
const {readdir} = require('fs').promises;
const https = require('https');
const {relative} = require('path');
const {RotationItemDef, TemplateDef} = require('./types');

Expand Down Expand Up @@ -399,7 +399,7 @@ function getSessionDateFromTitle(title) {
}
const [
// @ts-ignore
unusedFullMatch, // eslint-disable-line no-unused-vars
unusedFullMatch, // eslint-disable-line @typescript-eslint/no-unused-vars
day,
time,
] = match;
Expand Down
2 changes: 1 addition & 1 deletion build-system/global.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,4 @@ declare global {
}
}

export { }
export {};
2 changes: 1 addition & 1 deletion build-system/release-tagger/make-release.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const {
getRef,
} = require('./utils');
const {getExtensions, getSemver} = require('../npm-publish/utils');
const {GraphQlQueryResponseData} = require('@octokit/graphql'); //eslint-disable-line no-unused-vars
const {GraphQlQueryResponseData} = require('@octokit/graphql'); // eslint-disable-line @typescript-eslint/no-unused-vars

const prereleaseConfig = {
'beta-opt-in': true,
Expand Down
2 changes: 1 addition & 1 deletion build-system/release-tagger/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

const dedent = require('dedent');
const {GraphQlQueryResponseData, graphql} = require('@octokit/graphql'); //eslint-disable-line no-unused-vars
const {GraphQlQueryResponseData, graphql} = require('@octokit/graphql'); // eslint-disable-line @typescript-eslint/no-unused-vars
const {Octokit} = require('@octokit/rest');

// setup
Expand Down
2 changes: 1 addition & 1 deletion build-system/server/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,7 @@ function liveListInsert(liveList, node) {
*/
const child = /** @type {*} */ (node.cloneNode(true));
child.setAttribute('id', `list-item-${itemCtr++}`);
child.setAttribute('data-sort-time', Date.now());
child.setAttribute('data-sort-time', Date.now().toString());
liveList.querySelector('[items]')?.appendChild(child);
}
}
Expand Down
17 changes: 8 additions & 9 deletions build-system/server/new-server/transforms/cdn/cdn-transform.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@


import posthtml from 'posthtml';
import {
getCdnUrlAttr,
Expand All @@ -11,7 +9,10 @@ import {CDNURLToLocalHostRelativeAbsoluteDist} from '../utilities/cdn';
import {OptionSet} from '../utilities/option-set';
import {parse} from 'path';

function maybeModifyCdnUrl(node: posthtml.Node, options: OptionSet): posthtml.Node {
function maybeModifyCdnUrl(
node: posthtml.Node,
options: OptionSet
): posthtml.Node {
// Make sure to call `isJsonScript` before `tryGetUrl`. We bail out early if
// the node is of type="application/json" since it wouldn't have a URL.
if (isJsonScript(node)) {
Expand Down Expand Up @@ -50,11 +51,9 @@ export default function (
options: OptionSet = {}
): (tree: posthtml.Node) => void {
return function (tree: posthtml.Node) {
tree.match([
{tag: 'script'},
{tag: 'link', attrs: {rel: 'stylesheet'}},
], (node) => {
return maybeModifyCdnUrl(node, options);
});
tree.match(
[{tag: 'script'}, {tag: 'link', attrs: {rel: 'stylesheet'}}],
(node) => maybeModifyCdnUrl(node, options)
);
};
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import minimist from 'minimist';
import posthtml from 'posthtml';
import {readFileSync} from 'fs';
import {OptionSet} from '../utilities/option-set';
import {Lazy} from '../utilities/lazy';

const argv = minimist(process.argv.slice(2));
Expand Down Expand Up @@ -66,9 +65,7 @@ function prependAmpStyles(head: posthtml.Node): posthtml.Node {
/**
* Replace the src for every stories script tag.
*/
export default function (
_options: OptionSet = {}
): (tree: posthtml.Node) => void {
export default function (): (tree: posthtml.Node) => void {
return function (tree: posthtml.Node) {
let isAmp = false;
tree.match({tag: 'html'}, function (html: posthtml.Node): posthtml.Node {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,23 @@


import posthtml from 'posthtml';
import {isJsonScript, isValidScript, toExtension, ScriptNode, tryGetUrl} from '../utilities/cdn-tag';
import {
ScriptNode,
isJsonScript,
isValidScript,
toExtension,
tryGetUrl,
} from '../utilities/cdn-tag';
import {OptionSet} from '../utilities/option-set';

/**
* @param head
* @param script
*/
function appendModuleScript(head: posthtml.Node, nomoduleScript: ScriptNode, options: OptionSet): void {
const modulePath = toExtension(tryGetUrl(nomoduleScript.attrs.src), '.mjs').toString();
const moduleScript : ScriptNode = {
function appendModuleScript(
head: posthtml.Node,
nomoduleScript: ScriptNode,
options: OptionSet
): void {
const modulePath = toExtension(
tryGetUrl(nomoduleScript.attrs.src),
'.mjs'
).toString();
const moduleScript: ScriptNode = {
...nomoduleScript,
attrs: {
...nomoduleScript.attrs,
Expand All @@ -37,11 +44,13 @@ function appendModuleScript(head: posthtml.Node, nomoduleScript: ScriptNode, opt
* Returns a function that will transform script node sources into module/nomodule pair.
* @param options
*/
export default function(options: OptionSet = {}): (tree: posthtml.Node) => void {
return function(tree: posthtml.Node): void {
export default function (
options: OptionSet = {}
): (tree: posthtml.Node) => void {
return function (tree: posthtml.Node): void {
let head: posthtml.Node | undefined = undefined;
const scripts: Array<ScriptNode> = [];
tree.walk(node => {
tree.walk((node) => {
if (node.tag === 'head') {
head = node;
}
Expand All @@ -64,12 +73,13 @@ export default function(options: OptionSet = {}): (tree: posthtml.Node) => void
});

if (head === undefined) {
// eslint-disable-next-line local/no-forbidden-terms
console.log('Could not find a head element in the document');
return;
}

for (const script of scripts) {
appendModuleScript(head, script, options);
}
}
};
}
17 changes: 10 additions & 7 deletions build-system/server/new-server/transforms/sxg/sxg-transform.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@


import posthtml from 'posthtml';
import {URL} from 'url';
import {isJsonScript, isValidScript} from '../utilities/cdn-tag';
import {OptionSet} from '../utilities/option-set';

function sxgTransform(node: posthtml.Node, options: OptionSet = {}): posthtml.Node {
function sxgTransform(
node: posthtml.Node,
options: OptionSet = {}
): posthtml.Node {
// Make sure that isJsonScript is used before `isValidScript`. We bail out
// early if the ScriptNode is of type="application/json" since it wouldn't
// have any src url to modify.
Expand All @@ -18,7 +19,7 @@ function sxgTransform(node: posthtml.Node, options: OptionSet = {}): posthtml.No
}

if (options.minified) {
const src = node.attrs.src;
const {src} = node.attrs;
node.attrs.src = src.replace('.js', '.sxg.js');
} else {
const url = new URL(node.attrs.src);
Expand All @@ -33,10 +34,12 @@ function sxgTransform(node: posthtml.Node, options: OptionSet = {}): posthtml.No
* Returns a function that will transform script node sources into their sxg counterparts.
* @param options
*/
export default function(options: OptionSet = {}): (tree: posthtml.Node) => void {
return function(tree: posthtml.Node) {
export default function (
options: OptionSet = {}
): (tree: posthtml.Node) => void {
return function (tree: posthtml.Node) {
tree.match({tag: 'script'}, (script) => {
return sxgTransform(script, options);
});
}
};
}
13 changes: 3 additions & 10 deletions build-system/server/new-server/transforms/transform.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@


import fs from 'fs';
import minimist from 'minimist';
import posthtml from 'posthtml';
Expand All @@ -10,7 +8,7 @@ import transformCss from './css/css-transform';
const argv = minimist(process.argv.slice(2));
const FOR_TESTING = argv._.includes('integration');
// Use 9876 if running integration tests as this is the KARMA_SERVER_PORT
const PORT = FOR_TESTING ? 9876 : (argv.port ?? 8000);
const PORT = FOR_TESTING ? 9876 : argv.port ?? 8000;
const ESM = !!argv.esm;

const defaultTransformConfig = {
Expand All @@ -20,15 +18,10 @@ const defaultTransformConfig = {
useMaxNames: !argv.minified,
};

const transforms = [
transformCdnSrcs(defaultTransformConfig),
];
const transforms = [transformCdnSrcs(defaultTransformConfig)];

if (ESM) {
transforms.unshift(
transformCss(),
transformModules(defaultTransformConfig),
);
transforms.unshift(transformCss(), transformModules(defaultTransformConfig));
}

export async function transform(fileLocation: string): Promise<string> {
Expand Down
23 changes: 15 additions & 8 deletions build-system/server/new-server/transforms/utilities/cdn-tag.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@


import posthtml from 'posthtml';
import {URL} from 'url';
import {extname} from 'path';
import {extname, format, parse} from 'path';
import {VALID_CDN_ORIGIN} from './cdn';
import {parse, format} from 'path';

export interface ScriptNode extends posthtml.Node {
tag: 'script';
Expand All @@ -20,7 +17,10 @@ function isValidScriptExtension(url: URL): boolean {
return VALID_SCRIPT_EXTENSIONS.includes(extname(url.pathname));
}

export function isValidOrigin(url: URL, looseOriginUrlCheck?: boolean): boolean {
export function isValidOrigin(
url: URL,
looseOriginUrlCheck?: boolean
): boolean {
return looseOriginUrlCheck || url.origin === VALID_CDN_ORIGIN;
}

Expand All @@ -29,7 +29,7 @@ export function getCdnUrlAttr(node: posthtml.Node): string | null {
return 'src';
}
if (node.tag === 'link') {
return 'href'
return 'href';
}
return null;
}
Expand All @@ -38,7 +38,10 @@ export function getCdnUrlAttr(node: posthtml.Node): string | null {
* Determines if a Node is really a ScriptNode.
* @param node
*/
export function isValidScript(node: posthtml.Node, looseOriginUrlCheck?: boolean): node is ScriptNode {
export function isValidScript(
node: posthtml.Node,
looseOriginUrlCheck?: boolean
): node is ScriptNode {
if (node.tag !== 'script') {
return false;
}
Expand Down Expand Up @@ -73,6 +76,10 @@ export function toExtension(url: URL, extension: string): URL {
* This is a temporary measure to allow for a relaxed parsing of our
* fixture files' src urls before they are all fixed accordingly.
*/
export function tryGetUrl(src: string, host: string = '0.0.0.0', port: number = 8000): URL {
export function tryGetUrl(
src: string,
host: string = '0.0.0.0',
port: number = 8000
): URL {
return new URL(src, `http://${host}:${port}`);
}
Loading

0 comments on commit b2e9a06

Please sign in to comment.