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

feat: better bundling process and scope analysis #136

Open
wants to merge 4 commits 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
3 changes: 2 additions & 1 deletion eslint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export default pionxzh(
'**/node_modules/**',
'**/dist/**',
'packages/browserfs/**',
'packages/cli/test-*.js',
'packages/e2e/fixtures',
'packages/e2e/snapshots',
],
Expand All @@ -25,7 +26,7 @@ export default pionxzh(
},
},
{
files: ['packages/unminify/**/*.spec.ts'],
files: ['packages/**/*.spec.ts'],
rules: {
'style/indent': ['error', 2],
'no-restricted-syntax': [
Expand Down
11 changes: 4 additions & 7 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,22 +32,19 @@
"@vitest/ui": "^1.5.0",
"eslint": "^8.57.0",
"esno": "^0.17.0",
"globby": "^11.1.0",
"prettier": "^2.8.8",
"taze": "^0.13.6",
"turbo": "^1.13.2",
"typescript": "^5.4.5",
"typescript": "^5.5.4",
"vitest": "^1.5.0"
},
"pnpm": {
"patchedDependencies": {
"[email protected]": "patches/[email protected]",
"@clack/[email protected]": "patches/@[email protected]",
"@clack/[email protected]": "patches/@[email protected]",
"[email protected]": "patches/[email protected]"
"@clack/[email protected]": "patches/@[email protected]"
}
},
"resolutions": {
"ast-types": "^0.16.1"
"ast-types": "npm:[email protected]",
"recast": "npm:[email protected]"
}
}
28 changes: 14 additions & 14 deletions packages/ast-utils/__tests__/identifier.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,21 @@ import { expect, it } from 'vitest'
import { generateName as fn } from '../src/identifier'

it('should generate a identifier name', () => {
expect(fn('foo')).toBe('foo')
expect(fn('foo-bar')).toBe('fooBar')
expect(fn('foo.bar')).toBe('fooBar')
expect(fn('@foo/bar')).toBe('fooBar')
expect(fn('@foo/bar-baz')).toBe('fooBarBaz')
expect(fn('@foo/bar.baz')).toBe('fooBarBaz')
expect(fn('./foo')).toBe('foo')
expect(fn('./nested/foo')).toBe('nestedFoo')
expect(fn('./nested/foo-bar')).toBe('nestedFooBar')
expect(fn('./nested/foo.bar')).toBe('nestedFooBar')
expect(fn('../nested/foo-bar-baz')).toBe('nestedFooBarBaz')
expect(fn('../deep/nested/foo.bar.baz')).toBe('nestedFooBarBaz')
expect(fn('foo')).toBe('foo')
expect(fn('foo-bar')).toBe('fooBar')
expect(fn('foo.bar')).toBe('fooBar')
expect(fn('@foo/bar')).toBe('fooBar')
expect(fn('@foo/bar-baz')).toBe('fooBarBaz')
expect(fn('@foo/bar.baz')).toBe('fooBarBaz')
expect(fn('./foo')).toBe('foo')
expect(fn('./nested/foo')).toBe('nestedFoo')
expect(fn('./nested/foo-bar')).toBe('nestedFooBar')
expect(fn('./nested/foo.bar')).toBe('nestedFooBar')
expect(fn('../nested/foo-bar-baz')).toBe('nestedFooBarBaz')
expect(fn('../deep/nested/foo.bar.baz')).toBe('nestedFooBarBaz')
})

it('should generate a valid identifier name', () => {
expect(fn('import')).toBe('_import')
expect(fn('const')).toBe('_const')
expect(fn('import')).toBe('_import')
expect(fn('const')).toBe('_const')
})
80 changes: 69 additions & 11 deletions packages/ast-utils/__tests__/renameFunctionParameters.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,35 @@ import { defineInlineTest } from '@wakaru/test-utils'
import { renameFunctionParameters } from '../src/renameFunctionParameters'

const transform = createJSCodeshiftTransformationRule({
name: 'test-rename-function-parameters',
transform: (context) => {
const { root, j } = context

root
.find(j.FunctionDeclaration)
.forEach((path) => {
const node = path.node
renameFunctionParameters(j, node, ['c', 'd', 'xx', 'zz'])
})
},
name: 'test-rename-function-parameters',
transform: (context) => {
const { root, j } = context

root
.find(j.FunctionDeclaration)
.forEach((path) => {
const node = path.node
renameFunctionParameters(j, node, ['c', 'd', 'xx', 'zz'])
})
},
})

const transformWebpack = createJSCodeshiftTransformationRule({
name: 'test-rename-function-parameters-webpack',
transform: (context) => {
const { root, j } = context

root
.find(j.FunctionDeclaration)
.forEach((path) => {
const node = path.node
renameFunctionParameters(j, node, ['module', 'exports'])
})
},
})

const inlineTest = defineInlineTest(transform)
const inlineTestWebpack = defineInlineTest(transformWebpack)

inlineTest('should rename function parameters',
`
Expand Down Expand Up @@ -113,6 +128,49 @@ function z(c, d, xx) {
`,
)

inlineTestWebpack('should rename function parameters #4',
`
function fromEntries(U, B) {
"use strict";
function G() {
Object.fromEntries ||
(Object.fromEntries = function (U) {
for (var B = {}, G = 0, Y = U; G < Y.length; G++) {
var V = Y[G];
Object.defineProperty(B, V[0], {
configurable: !0,
enumerable: !0,
writable: !0,
value: V[1],
});
}
return B;
});
}
Object.defineProperty(B, "__esModule", { value: !0 }), (B.default = G);
}`
, `
function fromEntries(module, exports) {
"use strict";
function G() {
Object.fromEntries ||
(Object.fromEntries = function (U) {
for (var B = {}, G = 0, Y = U; G < Y.length; G++) {
var V = Y[G];
Object.defineProperty(B, V[0], {
configurable: !0,
enumerable: !0,
writable: !0,
value: V[1],
});
}
return B;
});
}
Object.defineProperty(exports, "__esModule", { value: !0 }), (exports.default = G);
}`,
)

inlineTest('Class',
`
function foo(a, b, x, z) {
Expand Down
11 changes: 6 additions & 5 deletions packages/ast-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,16 @@
"lint": "eslint src --max-warnings=0",
"lint:fix": "eslint src --fix --max-warnings=0"
},
"dependencies": {
"@babel/helper-validator-identifier": "^7.24.7",
"ast-types": "npm:[email protected]",
"jscodeshift": "npm:[email protected]"
},
"devDependencies": {
"@babel/helper-validator-identifier": "^7.22.20",
"@babel/types": "^7.24.0",
"@types/jscodeshift": "^0.11.11",
"@wakaru/ds": "workspace:*",
"@wakaru/shared": "workspace:*",
"@wakaru/test-utils": "workspace:*",
"ast-types": "^0.16.1",
"jscodeshift": "^0.15.2",
"typescript": "^5.4.5"
"typescript": "^5.5.4"
}
}
1 change: 0 additions & 1 deletion packages/ast-utils/src/comments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ export function removePureAnnotation(j: JSCodeshift, node: Node) {
return node
}

// /*#__PURE__*/
function isPureAnnotation(j: JSCodeshift, comment: CommentKind) {
return j.CommentBlock.check(comment)
&& comment.value.trim() === '#__PURE__'
Expand Down
30 changes: 28 additions & 2 deletions packages/ast-utils/src/identifier.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// @ts-expect-error no types
import { isIdentifierName, isKeyword, isStrictReservedWord } from '@babel/helper-validator-identifier'
import { toIdentifier } from '@babel/types'
import { isIdentifierChar, isIdentifierName, isKeyword, isStrictReservedWord } from '@babel/helper-validator-identifier'
import { isDeclared } from './scope'
import type { Scope } from 'ast-types/lib/scope'

Expand Down Expand Up @@ -74,3 +73,30 @@ function getUniqueName(name: string, scope: Scope | null = null, existedNames: s
}
return `${name}_${i}`
}

/**
* Fork from https://github.com/babel/babel/blob/main/packages/babel-types/src/converters/toIdentifier.ts
*/
function toIdentifier(input: string): string {
input = `${input}`

// replace all non-valid identifiers with dashes
let name = ''
for (const c of input) {
name += isIdentifierChar(c.codePointAt(0)) ? c : '-'
}

// remove all dashes and numbers from start of name
name = name.replace(/^[-0-9]+/, '')

// camel case
name = name.replace(/[-\s]+(.)?/g, (_match, c) => {
return c ? c.toUpperCase() : ''
})

if (!isValidIdentifier(name)) {
name = `_${name}`
}

return name || '_'
}
36 changes: 3 additions & 33 deletions packages/ast-utils/src/imports.ts
Original file line number Diff line number Diff line change
@@ -1,37 +1,7 @@
import { MultiMap } from '@wakaru/ds'
import { isTopLevel } from './isTopLevel'
import type { NodePath } from 'ast-types/lib/node-path'
import type { ASTNode, CallExpression, Collection, ImportDeclaration, JSCodeshift, StringLiteral, VariableDeclaration, VariableDeclarator } from 'jscodeshift'

type Source = string
type Imported = string
type Local = string

export interface DefaultImport {
type: 'default'
name: string
source: Source
}

export interface NamespaceImport {
type: 'namespace'
name: string
source: Source
}

export interface NamedImport {
type: 'named'
name: string
local: Local
source: Source
}

export interface BareImport {
type: 'bare'
source: Source
}

export type ImportInfo = DefaultImport | NamespaceImport | NamedImport | BareImport
import type { BareImport, DefaultImport, ImportInfo, Imported, Local, NamedImport, NamespaceImport, Source } from '@wakaru/shared/imports'
import type { ASTNode, ASTPath, CallExpression, Collection, ImportDeclaration, JSCodeshift, StringLiteral, VariableDeclaration, VariableDeclarator } from 'jscodeshift'

export class ImportManager {
private importSourceOrder = new Set<Source>()
Expand All @@ -40,7 +10,7 @@ export class ImportManager {
namedImports = new Map<Source, MultiMap<Imported, Local>>()
bareImports = new Set<Source>()

private importDecls: Array<NodePath<ImportDeclaration>> = []
private importDecls: Array<ASTPath<ImportDeclaration>> = []

get importMap() {
/**
Expand Down
3 changes: 1 addition & 2 deletions packages/ast-utils/src/matchers/isIIFE.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { fromPaths } from 'jscodeshift/src/Collection'
import type { ASTNode, ASTPath, CallExpression, Collection, ExpressionStatement, JSCodeshift, Statement } from 'jscodeshift'

/**
Expand Down Expand Up @@ -80,5 +79,5 @@ export function findIIFEs(
.map(path => (path.get('expression', 'argument') as ASTPath<CallExpression>))
.paths()

return fromPaths([...collection1, ...collection2])
return j([...collection1, ...collection2])
}
8 changes: 7 additions & 1 deletion packages/ast-utils/src/reference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ export function findReferences(
): Collection<Identifier> {
const targetScope = 'bindings' in nodeOrScope ? nodeOrScope : j(nodeOrScope).get().scope as Scope
const range = 'bindings' in nodeOrScope ? nodeOrScope.path : nodeOrScope
const rangeNode = 'node' in range ? range.node : range

return j(range)
.find(j.Identifier, { name: identifierName })
Expand All @@ -179,9 +180,14 @@ export function findReferences(
// ignore properties (e.g. in MemberExpression
if (path.name === 'property' && j.MemberExpression.check(path.parent.node) && !path.parent.node.computed) return false

// ignore function name that is at the top level
if (path.parent.node === rangeNode && j.FunctionDeclaration.check(path.parent.node) && path.parent.node.id === path.node) {
return false
}

if (!path.scope) return false

let scope = path.scope
let scope: Scope | null = path.scope
// we don't use `scope.lookup` here to avoid
// traversing the whole scope chain
while (scope && scope !== targetScope) {
Expand Down
11 changes: 11 additions & 0 deletions packages/ast-utils/src/renameFunctionParameters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,17 @@ export function renameFunctionParameters(j: JSCodeshift, node: FunctionDeclarati
const newName = parameters[index]
if (!newName || oldName === newName) return

/**
* Skip if the old name is declared multiple times
* it means the parameter is shadowed by another variable
* in the same scope
*/
const bindings = targetScope.getBindings()
if (bindings[oldName]?.length > 1) {
param.name = newName
return
}

renameIdentifier(j, targetScope, oldName, newName)
}
})
Expand Down
7 changes: 3 additions & 4 deletions packages/ast-utils/src/scope.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { fromPaths } from 'jscodeshift/src/Collection'
import { mergeComments } from './comments'
import { findReferences } from './reference'
import type { Scope } from 'ast-types/lib/scope'
import type { ASTPath, Collection, Identifier, ImportDeclaration, JSCodeshift, Statement, VariableDeclaration } from 'jscodeshift'

export function isDeclared(scope: Scope, name: string) {
export function isDeclared(scope: Scope | null, name: string) {
while (scope) {
if (scope.declares(name)) return true
scope = scope.parent
Expand All @@ -17,8 +16,8 @@ export function findDeclaration(scope: Scope, name: string): ASTPath<Identifier>
return scope.lookup(name)?.getBindings()[name]?.[0]
}

export function findDeclarations(scope: Scope, name: string): Collection<Identifier> {
return fromPaths(scope.lookup(name)?.getBindings()[name] ?? [])
export function findDeclarations(j: JSCodeshift, scope: Scope, name: string): Collection<Identifier> {
return j(scope.lookup(name)?.getBindings()[name] ?? [])
}

export function removeDeclarationIfUnused(j: JSCodeshift, path: ASTPath, name: string) {
Expand Down
2 changes: 1 addition & 1 deletion packages/ast-utils/src/types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { ImportInfo } from './imports'
import type { ImportInfo } from '@wakaru/shared/imports'

export type ModuleMapping = Record<number | string, string>

Expand Down
4 changes: 2 additions & 2 deletions packages/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
"@wakaru/unpacker": "workspace:*",
"fs-extra": "^11.2.0",
"globby": "^11.1.0",
"picocolors": "^1.0.0",
"picocolors": "^1.0.1",
"poolifier": "^3.1.30",
"yargs": "^17.7.2"
},
Expand All @@ -42,6 +42,6 @@
"@wakaru/shared": "workspace:*",
"@wakaru/test-utils": "workspace:*",
"tsup": "^8.0.2",
"typescript": "^5.4.5"
"typescript": "^5.5.4"
}
}
Loading