Skip to content

Commit

Permalink
Adding conformance checks (#10314)
Browse files Browse the repository at this point in the history
* adding tests  for rect sync conformance check

* adding test for react sync script conformance check

* reverting yarn lock changes

* adding duplicate polyfill conformance

* bug fixes in dulpicate polyfill conformance check

* adding settings capability to conformance plugin

* removing minification check from server build

* bug fix

* settings for react sync script check
  • Loading branch information
prateekbh committed Mar 2, 2020
1 parent 4056e98 commit 16672a4
Show file tree
Hide file tree
Showing 17 changed files with 486 additions and 8 deletions.
41 changes: 40 additions & 1 deletion packages/next/build/webpack-config.ts
Expand Up @@ -44,6 +44,8 @@ import { ServerlessPlugin } from './webpack/plugins/serverless-plugin'
import { TerserPlugin } from './webpack/plugins/terser-webpack-plugin/src/index'
import WebpackConformancePlugin, {
MinificationConformanceCheck,
ReactSyncScriptsConformanceCheck,
DuplicatePolyfillsConformanceCheck,
} from './webpack/plugins/webpack-conformance-plugin'

type ExcludesFalse = <T>(x: T | false) => x is T
Expand Down Expand Up @@ -425,6 +427,26 @@ export default async function getBaseWebpackConfig(
customAppFile = path.resolve(path.join(pagesDir, customAppFile))
}

const conformanceConfig = Object.assign(
{
ReactSyncScriptsConformanceCheck: {
enabled: true,
},
MinificationConformanceCheck: {
enabled: true,
},
DuplicatePolyfillsConformanceCheck: {
enabled: true,
BlockedAPIToBePolyfilled: Object.assign(
[],
['fetch'],
config.conformance?.DuplicatePolyfillsConformanceCheck
?.BlockedAPIToBePolyfilled || []
),
},
},
config.conformance
)
let webpackConfig: webpack.Configuration = {
externals: !isServer
? undefined
Expand Down Expand Up @@ -844,7 +866,24 @@ export default async function getBaseWebpackConfig(
config.experimental.conformance &&
!dev &&
new WebpackConformancePlugin({
tests: [new MinificationConformanceCheck()],
tests: [
!isServer &&
conformanceConfig.MinificationConformanceCheck.enabled &&
new MinificationConformanceCheck(),
conformanceConfig.ReactSyncScriptsConformanceCheck.enabled &&
new ReactSyncScriptsConformanceCheck({
AllowedSources:
conformanceConfig.ReactSyncScriptsConformanceCheck
.allowedSources || [],
}),
!isServer &&
conformanceConfig.DuplicatePolyfillsConformanceCheck.enabled &&
new DuplicatePolyfillsConformanceCheck({
BlockedAPIToBePolyfilled:
conformanceConfig.DuplicatePolyfillsConformanceCheck
.BlockedAPIToBePolyfilled,
}),
].filter(Boolean),
}),
].filter((Boolean as any) as ExcludesFalse),
}
Expand Down
@@ -0,0 +1,210 @@
import {
IWebpackConformanceTest,
IGetAstNodeResult,
IParsedModuleDetails,
IConformanceTestResult,
IConformanceTestStatus,
} from '../TestInterface'
import {
CONFORMANCE_ERROR_PREFIX,
CONFORMANCE_WARNING_PREFIX,
} from '../constants'
import { types } from 'recast'
import { NodePath } from 'ast-types/lib/node-path'
import { namedTypes } from 'ast-types'
import { getLocalFileName } from '../utils/file-utils'
import {
isNodeCreatingScriptElement,
reducePropsToObject,
} from '../utils/ast-utils'

function getMessage(
property: string,
request: string,
isWarning: Boolean = false
): string {
if (isWarning) {
return `${CONFORMANCE_WARNING_PREFIX}: Found a ${property} polyfill in ${getLocalFileName(
request
)}.`
}
return `${CONFORMANCE_ERROR_PREFIX}: Found a ${property} polyfill in ${getLocalFileName(
request
)}.`
}

export interface DuplicatePolyfillsConformanceTestSettings {
BlockedAPIToBePolyfilled?: string[]
}

const BANNED_LEFT_OBJECT_TYPES = ['Identifier', 'ThisExpression']

export class DuplicatePolyfillsConformanceCheck
implements IWebpackConformanceTest {
private BlockedAPIs: string[] = []
constructor(options: DuplicatePolyfillsConformanceTestSettings = {}) {
this.BlockedAPIs = options.BlockedAPIToBePolyfilled || []
}
public getAstNode(): IGetAstNodeResult[] {
const EARLY_EXIT_SUCCESS_RESULT: IConformanceTestResult = {
result: IConformanceTestStatus.SUCCESS,
}
return [
{
visitor: 'visitAssignmentExpression',
inspectNode: (
path: NodePath<namedTypes.AssignmentExpression>,
{ request }: IParsedModuleDetails
): IConformanceTestResult => {
const { node } = path
const left = node.left as namedTypes.MemberExpression
/**
* We're only interested in code like `foo.fetch = bar;`.
* For anything else we exit with a success.
* Also foo in foo.bar needs to be either Identifier or `this` and not someFunction().fetch;
*/
if (
left.type !== 'MemberExpression' ||
!BANNED_LEFT_OBJECT_TYPES.includes(left.object.type) ||
left.property.type !== 'Identifier'
) {
return EARLY_EXIT_SUCCESS_RESULT
}
if (!this.BlockedAPIs.includes(left.property.name)) {
return EARLY_EXIT_SUCCESS_RESULT
}
/**
* Here we know the code is `foo.(fetch/URL) = something.
* If foo === this/self, fail it immediately.
* check for this.[fetch|URL(...BlockedAPIs)]/ self.[fetch|URL(...BlockedAPIs)]
**/
if (isNodeThisOrSelf(left.object)) {
return {
result: IConformanceTestStatus.FAILED,
warnings: [
{
message: getMessage(left.property.name, request),
},
],
}
}
/**
* we now are sure the code under examination is
* `globalVar.[fetch|URL(...BlockedAPIs)] = something`
**/
const objectName = (left.object as namedTypes.Identifier).name
const allBindings = path.scope.lookup(objectName)
if (!allBindings) {
/**
* we have absolutely no idea where globalVar came from,
* so lets just exit
**/
return EARLY_EXIT_SUCCESS_RESULT
}

try {
const sourcePath = allBindings.bindings[objectName][0]
const originPath = sourcePath.parentPath
const {
node: originNode,
}: { node: namedTypes.VariableDeclarator } = originPath
if (
originNode.type === 'VariableDeclarator' &&
isNodeThisOrSelf(originNode.init)
) {
return {
result: IConformanceTestStatus.FAILED,
warnings: [
{
message: getMessage(left.property.name, request),
},
],
}
}
if (
originPath.name === 'params' &&
originPath.parentPath.firstInStatement()
) {
/**
* We do not know what will be the value of this param at runtime so we just throw a warning.
* ```
* (function(scope){
* ....
* scope.fetch = new Fetch();
* })(.....)
* ```
*/
return {
result: IConformanceTestStatus.FAILED,
warnings: [
{
message: getMessage(left.property.name, request, true),
},
],
}
}
} catch (e) {
return EARLY_EXIT_SUCCESS_RESULT
}

return EARLY_EXIT_SUCCESS_RESULT
},
},
{
visitor: 'visitCallExpression',
inspectNode: (path: NodePath, { request }: IParsedModuleDetails) => {
const { node }: { node: types.namedTypes.CallExpression } = path
if (!node.arguments || node.arguments.length < 2) {
return EARLY_EXIT_SUCCESS_RESULT
}
if (isNodeCreatingScriptElement(node)) {
const propsNode = node
.arguments[1] as types.namedTypes.ObjectExpression
if (!propsNode.properties) {
return EARLY_EXIT_SUCCESS_RESULT
}
const props: {
[key: string]: string
} = reducePropsToObject(propsNode)
if (!('src' in props)) {
return EARLY_EXIT_SUCCESS_RESULT
}
const foundBannedPolyfill = doesScriptLoadBannedAPIfromPolyfillIO(
props.src,
this.BlockedAPIs
)
if (foundBannedPolyfill) {
return {
result: IConformanceTestStatus.FAILED,
warnings: [
{
message: `${CONFORMANCE_WARNING_PREFIX}: Found polyfill.io loading polyfill for ${foundBannedPolyfill}.`,
},
],
}
}
}
return EARLY_EXIT_SUCCESS_RESULT
},
},
]
}
}

function isNodeThisOrSelf(node: any): boolean {
return (
node.type === 'ThisExpression' ||
(node.type === 'Identifier' && node.name === 'self')
)
}

function doesScriptLoadBannedAPIfromPolyfillIO(
source: string,
blockedAPIs: string[]
): string | undefined {
const url = new URL(source)
if (url.hostname === 'polyfill.io' && url.searchParams.has('features')) {
const requestedAPIs = (url.searchParams.get('features') || '').split(',')
return blockedAPIs.find(api => requestedAPIs.includes(api))
}
}
Expand Up @@ -4,12 +4,22 @@ import {
IConformanceTestStatus,
} from '../TestInterface'
import { CONFORMANCE_ERROR_PREFIX } from '../constants'
const EARLY_EXIT_RESULT: IConformanceTestResult = {
result: IConformanceTestStatus.SUCCESS,
}

export class MinificationConformanceCheck implements IWebpackConformanceTest {
public buildStared(options: any): IConformanceTestResult {
if (options.output.path.endsWith('/server')) {
return EARLY_EXIT_RESULT
}
// TODO(prateekbh@): Implement warning for using Terser maybe?

if (options.optimization.minimize === false) {
const { optimization } = options
if (
optimization &&
(optimization.minimize !== true ||
(optimization.minimizer && optimization.minimizer.length === 0))
) {
return {
result: IConformanceTestStatus.FAILED,
errors: [
Expand All @@ -19,9 +29,7 @@ export class MinificationConformanceCheck implements IWebpackConformanceTest {
],
}
} else {
return {
result: IConformanceTestStatus.SUCCESS,
}
return EARLY_EXIT_RESULT
}
}
}
@@ -0,0 +1,84 @@
import {
IWebpackConformanceTest,
IGetAstNodeResult,
IParsedModuleDetails,
IConformanceTestResult,
IConformanceTestStatus,
} from '../TestInterface'
import {
CONFORMANCE_ERROR_PREFIX,
CONFORMANCE_WARNING_PREFIX,
} from '../constants'
import { namedTypes } from 'ast-types/'
import { NodePath } from 'ast-types/lib/node-path'
import { getLocalFileName } from '../utils/file-utils'
import { isNodeCreatingScriptElement } from '../utils/ast-utils'
export const ErrorMessage: string = `${CONFORMANCE_ERROR_PREFIX}: A sync script was found in a react module.`
export const WarningMessage: string = `${CONFORMANCE_WARNING_PREFIX}: A sync script was found in a react module.`
export const ErrorDescription = ``
const EARLY_EXIT_SUCCESS_RESULT: IConformanceTestResult = {
result: IConformanceTestStatus.SUCCESS,
}

export interface ReactSyncScriptsConformanceCheckOptions {
AllowedSources?: String[]
}
export class ReactSyncScriptsConformanceCheck
implements IWebpackConformanceTest {
private allowedSources: String[] = []
constructor({
AllowedSources,
}: ReactSyncScriptsConformanceCheckOptions = {}) {
if (AllowedSources) {
this.allowedSources = AllowedSources
}
}

public getAstNode(): IGetAstNodeResult[] {
return [
{
visitor: 'visitCallExpression',
inspectNode: (path: NodePath, { request }: IParsedModuleDetails) => {
const { node }: { node: namedTypes.CallExpression } = path
if (!node.arguments || node.arguments.length < 2) {
return EARLY_EXIT_SUCCESS_RESULT
}
if (isNodeCreatingScriptElement(node)) {
const propsNode = node.arguments[1] as namedTypes.ObjectExpression
if (!propsNode.properties) {
return EARLY_EXIT_SUCCESS_RESULT
}
const props: {
[key: string]: string
} = propsNode.properties.reduce((originalProps, prop: any) => {
// @ts-ignore
originalProps[prop.key.name] = prop.value.value
return originalProps
}, {})
if (
'defer' in props ||
'async' in props ||
!('src' in props) ||
this.allowedSources.includes(props.src)
) {
return EARLY_EXIT_SUCCESS_RESULT
}

// Todo: Add an absolute error case for modern js when class is a subclass of next/head.
return {
result: IConformanceTestStatus.FAILED,
warnings: [
{
message: `${WarningMessage} ${getLocalFileName(
request
)}. This can potentially delay FCP/FP metrics.`,
},
],
}
}
return EARLY_EXIT_SUCCESS_RESULT
},
},
]
}
}

0 comments on commit 16672a4

Please sign in to comment.