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

fix: missing types for es-module-lexer (fixes #8349) #8352

Merged
merged 2 commits into from May 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CONTRIBUTING.md
Expand Up @@ -214,6 +214,8 @@ Vite aims to be fully usable as a dependency in a TypeScript project (e.g. it sh

To get around this, we inline some of these dependencies' types in `packages/vite/types`. This way we can still expose the typing but bundle the dependency's source code.

Use `pnpm run check-dist-types` to check bundled types does not rely on types in `devDependencies`. If you are adding `dependencies`, make sure to configure `tsconfig.check.json`.

### Think before adding yet another option

We already have many config options, and we should avoid fixing an issue by adding yet another one. Before adding an option, try to think about:
Expand Down
3 changes: 2 additions & 1 deletion packages/vite/package.json
Expand Up @@ -49,10 +49,11 @@
"dev": "rimraf dist && pnpm run build-bundle -w",
"build": "rimraf dist && run-s build-bundle build-types",
"build-bundle": "rollup --config rollup.config.ts --configPlugin typescript",
"build-types": "run-s build-temp-types patch-types roll-types",
"build-types": "run-s build-temp-types patch-types roll-types check-dist-types",
"build-temp-types": "tsc --emitDeclarationOnly --outDir temp/node -p src/node",
"patch-types": "esno scripts/patchTypes.ts",
"roll-types": "api-extractor run && rimraf temp",
"check-dist-types": "tsc --project tsconfig.check.json",
"lint": "eslint --ext .ts src/**",
"format": "prettier --write --parser typescript \"src/**/*.ts\"",
"prepublishOnly": "npm run build"
Expand Down
2 changes: 2 additions & 0 deletions packages/vite/src/node/index.ts
Expand Up @@ -37,6 +37,8 @@ export type {
DepOptimizationProcessing,
OptimizedDepInfo,
DepsOptimizer,
EsModuleLexerImportSpecifier,
EsModuleLexerParseReturnType,
ExportsData
} from './optimizer'
export type { Plugin } from './plugin'
Expand Down
9 changes: 8 additions & 1 deletion packages/vite/src/node/optimizer/index.ts
Expand Up @@ -6,6 +6,7 @@ import colors from 'picocolors'
import type { BuildOptions as EsbuildBuildOptions } from 'esbuild'
import { build } from 'esbuild'
import { init, parse } from 'es-module-lexer'
import type { ImportSpecifier as EsModuleLexerImportSpecifier } from 'types/es-module-lexer'
import type { ResolvedConfig } from '../config'
import {
createDebugger,
Expand All @@ -31,7 +32,13 @@ const isDebugEnabled = _debug('vite:deps').enabled
const jsExtensionRE = /\.js$/i
const jsMapExtensionRE = /\.js\.map$/i

export type ExportsData = ReturnType<typeof parse> & {
export type { EsModuleLexerImportSpecifier }
export type EsModuleLexerParseReturnType = readonly [
imports: ReadonlyArray<EsModuleLexerImportSpecifier>,
exports: ReadonlyArray<string>,
facade: boolean
]
export type ExportsData = EsModuleLexerParseReturnType & {
Copy link
Member Author

Choose a reason for hiding this comment

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

These needs to be exported because it is included in the return type of vite.optimize.
But are these needs to be exported? (I didnt check closely yet)

Copy link
Member

Choose a reason for hiding this comment

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

We could also export them to be safe, and then we remove them in another PR if they are unneeded.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll check it and create a new PR if I felt it won't be needed 👍

// es-module-lexer has a facade detection but isn't always accurate for our
// use case when the module has default export
hasReExports?: true
Expand Down
20 changes: 20 additions & 0 deletions packages/vite/tsconfig.check.json
@@ -0,0 +1,20 @@
{
"compilerOptions": {
"noEmit": true,
"moduleResolution": "classic",
// Only add entries to `paths` when you are adding/updating dependencies (not devDependencies)
// See CONTRIBUTING.md "Ensure type support" for more details
"paths": {
// direct
"rollup": ["./node_modules/rollup/dist/rollup.d.ts"],
// direct
"esbuild": ["./node_modules/esbuild/lib/main.d.ts"],
// direct
"postcss": ["./node_modules/postcss/lib/postcss.d.ts"],
// indirect: postcss depends on it
"source-map-js": ["./node_modules/source-map-js/source-map.d.ts"]
},
"typeRoots": []
},
"include": ["dist/**/*.d.ts"]
}
90 changes: 90 additions & 0 deletions packages/vite/types/es-module-lexer.d.ts
@@ -0,0 +1,90 @@
// Modified and inlined to avoid extra dependency
// Source: https://github.com/guybedford/es-module-lexer/blob/main/types/lexer.d.ts
// MIT Licensed https://github.com/guybedford/es-module-lexer/blob/main/LICENSE

export interface ImportSpecifier {
/**
* Module name
*
* To handle escape sequences in specifier strings, the .n field of imported specifiers will be provided where possible.
*
* For dynamic import expressions, this field will be empty if not a valid JS string.
*
* @example
* const [imports1, exports1] = parse(String.raw`import './\u0061\u0062.js'`);
* imports1[0].n;
* // Returns "./ab.js"
*
* const [imports2, exports2] = parse(`import("./ab.js")`);
* imports2[0].n;
* // Returns "./ab.js"
*
* const [imports3, exports3] = parse(`import("./" + "ab.js")`);
* imports3[0].n;
* // Returns undefined
*/
readonly n: string | undefined
/**
* Start of module specifier
*
* @example
* const source = `import { a } from 'asdf'`;
* const [imports, exports] = parse(source);
* source.substring(imports[0].s, imports[0].e);
* // Returns "asdf"
*/
readonly s: number
/**
* End of module specifier
*/
readonly e: number

/**
* Start of import statement
*
* @example
* const source = `import { a } from 'asdf'`;
* const [imports, exports] = parse(source);
* source.substring(imports[0].ss, imports[0].se);
* // Returns `"import { a } from 'asdf';"`
*/
readonly ss: number
/**
* End of import statement
*/
readonly se: number

/**
* If this import statement is a dynamic import, this is the start value.
* Otherwise this is `-1`.
*/
readonly d: number

/**
* If this import has an import assertion, this is the start value.
* Otherwise this is `-1`.
*/
readonly a: number
}

/**
* Wait for init to resolve before calling `parse`.
*/
export const init: Promise<void>

/**
* Outputs the list of exports and locations of import specifiers,
* including dynamic import and import meta handling.
*
* @param source - Source code to parser
* @param name - Optional sourcename
* @returns Tuple contaning imports list and exports list.
*/
export function parse(
source: string,
name?: string
): readonly [
imports: ReadonlyArray<ImportSpecifier>,
exports: ReadonlyArray<string>,
facade: boolean
]