Skip to content

Commit

Permalink
fix(code splitting): infinite loop on cyclic imports
Browse files Browse the repository at this point in the history
  • Loading branch information
Anidetrix committed May 22, 2020
1 parent 16c1917 commit 4dd0ca3
Show file tree
Hide file tree
Showing 17 changed files with 60 additions and 61 deletions.
2 changes: 1 addition & 1 deletion __tests__/importer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describe("importer", () => {

it("warns about incorrect resolving", async () => {
const warning = await validateImport('@import "smh.css"', {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
// eslint-disable-next-line @typescript-eslint/no-explicit-any,@typescript-eslint/no-unsafe-return
resolve: () => "lol" as any,
});
expect(warning).toMatchSnapshot("warning");
Expand Down
2 changes: 1 addition & 1 deletion __tests__/url-resolver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ describe("url resolver", () => {

it("warns about incorrect resolving", async () => {
const warning = await validateUrl(".foo{background:url(bg.png)}", {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
// eslint-disable-next-line @typescript-eslint/no-explicit-any,@typescript-eslint/no-unsafe-return
resolve: () => "lol" as any,
});
expect(warning).toMatchSnapshot("warning");
Expand Down
1 change: 1 addition & 0 deletions __tests__/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ test("noop", async () => {
});

describe("load-module", () => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
const loadModule = jest.requireActual("../src/utils/load-module")
.default as typeof loadModuleMock;

Expand Down
38 changes: 18 additions & 20 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import {
} from "./utils/options";

export default (options: Options = {}): Plugin => {
const filter = createFilter(options.include, options.exclude);
const isIncluded = createFilter(options.include, options.exclude);

const postcssOpts: PostCSSLoaderOptions = {
...inferModeOption(options.mode),
Expand Down Expand Up @@ -62,8 +62,6 @@ export default (options: Options = {}): Plugin => {
extensions: postcssOpts.extensions,
});

const isSupported = (id: string): boolean => filter(id) && loaders.isSupported.call(loaders, id);

const extracted = new Map<string, NonNullable<Payload["extracted"]>>();

let preserveModules: boolean;
Expand All @@ -76,7 +74,7 @@ export default (options: Options = {}): Plugin => {
},

async transform(code, id) {
if (!isSupported(id)) return null;
if (!isIncluded(id) || !loaders.isSupported(id)) return null;

// Check if file was already processed into JS
// by other instance(s) of this or other plugin(s)
Expand Down Expand Up @@ -176,7 +174,7 @@ export default (options: Options = {}): Plugin => {

const entries = [...extracted.values()]
.filter(e => ids.includes(e.id))
.sort((a, b) => ids.indexOf(a.id) - ids.indexOf(b.id));
.sort((a, b) => ids.lastIndexOf(a.id) - ids.lastIndexOf(b.id));

const concat = new Concat(true, path.basename(fileName), "\n");
for (const res of entries) {
Expand All @@ -194,22 +192,22 @@ export default (options: Options = {}): Plugin => {
};

const getImports = (chunk: OutputChunk): string[] => {
const orderedIds = new Set<string>();

let ids = Object.keys(chunk.modules).reduce<string[]>((acc, id) => {
const i = this.getModuleInfo(id);
return [...acc, i.id, ...i.importedIds];
}, []);

for (;;) {
ids = ids.reduce<string[]>((acc, id) => {
const i = this.getModuleInfo(id);
if (isSupported(i.id)) orderedIds.delete(i.id), orderedIds.add(i.id);
return [...acc, ...i.importedIds];
}, []);

if (ids.length === 0) return [...orderedIds];
const ordered: string[] = [];

for (const module of Object.keys(chunk.modules)) {
const traversed = new Set<string>();
let ids = [module];
while (ids.length > 0) {
ids = ids.reduce<string[]>((acc, id) => {
if (!isIncluded(id) || traversed.has(id)) return acc;
if (extracted.has(id)) ordered.push(id);
else traversed.add(id);
return [...acc, ...this.getModuleInfo(id).importedIds];
}, []);
}
}

return ordered;
};

const getEmitted = (): Map<string, string[]> => {
Expand Down
4 changes: 2 additions & 2 deletions src/loaders/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const workQueue = new PQueue({ concurrency: threadPoolSize - 1 });

export default class Loaders {
loaders: Loader[] = [];
use: [string, object][];
use: [string, Record<string, unknown>][];
test: (file: string) => boolean;

constructor(options: LoadersOptions) {
Expand All @@ -46,7 +46,7 @@ export default class Loaders {
return this.loaders.find(loader => loader.name === name);
}

listLoader<T extends object>(loader: Loader<T>): void {
listLoader<T extends Record<string, unknown>>(loader: Loader<T>): void {
if (!this.use.some(rule => rule[0] === loader.name)) return;
if (this.getLoader(loader.name)) this.unlistLoader(loader.name);
this.loaders.push(loader as Loader);
Expand Down
2 changes: 1 addition & 1 deletion src/loaders/postcss/icss/load.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const load: Load = async (url, file, extensions, processor, opts) => {
const { messages } = await processor.process(source, { ...opts, from });
return messages
.filter(msg => msg.type === "icss")
.reduce((acc, msg) => ({ ...acc, ...msg.replacements }), {});
.reduce((acc, msg) => ({ ...acc, ...(msg.replacements as Record<string, string>) }), {});
};

export default load;
2 changes: 1 addition & 1 deletion src/loaders/postcss/import/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export interface ImportOptions {
* Overrides the global `alias` option.
* - ex.: `{"foo":"bar"}`
*/
alias?: { [from: string]: string };
alias?: Record<string, string>;
/**
* Import files ending with these extensions.
* Overrides the global `extensions` option.
Expand Down
6 changes: 3 additions & 3 deletions src/loaders/postcss/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ async function loadConfig(
const configPath =
typeof config === "object" && config.path ? path.resolve(config.path) : path.dirname(id);

const context: { [x: string]: object | string } = {
const context: Record<string, Record<string, unknown>> = {
file: {
extname: path.extname(id),
dirname: path.dirname(id),
Expand All @@ -42,7 +42,7 @@ async function loadConfig(
};

return loadPostCSSConfig(context, configPath).catch(error => {
if (!/no postcss config found/i.test(error.message)) throw error;
if (!/no postcss config found/i.test((error as Error).message)) throw error;
return {};
});
}
Expand Down Expand Up @@ -74,7 +74,7 @@ const loader: Loader<PostCSSLoaderOptions> = {

const supportModules = Boolean(options.modules || autoModules);

const modulesExports: { [file: string]: { [x: string]: string } } = {};
const modulesExports: Record<string, Record<string, string>> = {};

const postcssOpts: PostCSSLoaderOptions["postcss"] & {
from: string;
Expand Down
2 changes: 1 addition & 1 deletion src/loaders/postcss/modules/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export interface ModulesOptions {
*/
generateScopedName?: string | ((name: string, file: string, css: string) => string);
/** Function for resulting replacements extraction */
getReplacements?: (file: string, replacements: { [x: string]: string }, out?: string) => void;
getReplacements?: (file: string, replacements: Record<string, string>, out?: string) => void;
}

export default (options: ModulesOptions): postcss.Transformer[] => {
Expand Down
4 changes: 2 additions & 2 deletions src/loaders/postcss/url/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export interface UrlOptions {
* Overrides the global `alias` option.
* - ex.: `{"foo":"bar"}`
*/
alias?: { [from: string]: string };
alias?: Record<string, string>;
}

const plugin: postcss.Plugin<UrlOptions> = postcss.plugin(
Expand Down Expand Up @@ -87,7 +87,7 @@ const plugin: postcss.Plugin<UrlOptions> = postcss.plugin(

const imported = res.messages
.filter(msg => msg.type === "dependency")
.map<string>(msg => msg.file);
.map(msg => msg.file as string);

css.walkDecls(decl => {
if (!isDeclWithUrl(decl)) return;
Expand Down
2 changes: 1 addition & 1 deletion src/loaders/sourcemap.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Loader } from "../types";
import { getMap, stripMap } from "../utils/sourcemap";

const loader: Loader<{}> = {
const loader: Loader = {
name: "sourcemap",
alwaysProcess: true,
async process({ code, map }) {
Expand Down
4 changes: 2 additions & 2 deletions src/shims/less.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ declare namespace less {
math?: "always" | "strict" | "parens-division" | "parens" | "strict-legacy" | number;
silent?: boolean;
strictUnits?: boolean;
globalVars?: { [x: string]: string };
modifyVars?: { [x: string]: string };
globalVars?: Record<string, string>;
modifyVars?: Record<string, string>;
syncImport?: boolean;
}

Expand Down
2 changes: 1 addition & 1 deletion src/shims/postcss-modules.d.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable import/no-duplicates */
declare module "postcss-modules-values" {
import { Plugin } from "postcss";
export default function (options?: {}): Plugin<{}>;
export default function (options?: unknown): Plugin<unknown>;
}

declare module "postcss-modules-local-by-default" {
Expand Down
33 changes: 17 additions & 16 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ export interface PostCSSLoadConfigOptions {
* Context object passed to PostCSS config file
* @default {}
*/
ctx?: object;
ctx?: Record<string, unknown>;
}

/** Options for PostCSS loader */
export interface PostCSSLoaderOptions {
export interface PostCSSLoaderOptions extends Record<string, unknown> {
/** @see {@link Options.minimize} */
minimize: Exclude<Options["minimize"], true | undefined>;
/** @see {@link Options.config} */
Expand Down Expand Up @@ -59,7 +59,7 @@ export interface PostCSSLoaderOptions {
}

/** Options for Sass loader */
export interface SASSLoaderOptions {
export interface SASSLoaderOptions extends Record<string, unknown> {
/** Sass importer, or array of such */
importer?: sass.Importer | sass.Importer[];
/** Data to prepend to every Sass file */
Expand All @@ -68,30 +68,24 @@ export interface SASSLoaderOptions {
impl?: string;
/** Forcefully disable/enable `fibers` */
fibers?: boolean;
/** Any options for `sass` processor */
[option: string]: unknown;
}

/** Options for Stylus loader */
export interface StylusLoaderOptions {
export interface StylusLoaderOptions extends Record<string, unknown> {
/** Array of paths for Stylus */
paths?: string[];
/** Any options for `stylus` processor */
[option: string]: unknown;
}

/** Options for Less loader */
export interface LESSLoaderOptions {
export interface LESSLoaderOptions extends Record<string, unknown> {
/** Array of Less plugins */
plugins?: less.Plugin[];
/** Any options for `less` processor */
[option: string]: unknown;
}

/** Options for {@link Loaders} class */
export interface LoadersOptions {
/** @see {@link Options.use} */
use: (string | [string] | [string, object])[];
use: (string | [string] | [string, Record<string, unknown>])[];
/** @see {@link Options.loaders} */
loaders: Loader[];
/** @see {@link Options.extensions} */
Expand All @@ -102,7 +96,7 @@ export interface LoadersOptions {
* Loader
* @param TLoaderOptions type of loader's options
*/
export interface Loader<TLoaderOptions = object> {
export interface Loader<TLoaderOptions = Record<string, unknown>> {
/** Name */
name: string;
/**
Expand All @@ -120,7 +114,7 @@ export interface Loader<TLoaderOptions = object> {
* Loader's context
* @param TLoaderOptions type of loader's options
*/
export interface LoaderContext<TLoaderOptions = object> {
export interface LoaderContext<TLoaderOptions = Record<string, unknown>> {
/**
* Loader's options
* @default {}
Expand Down Expand Up @@ -201,7 +195,14 @@ export interface Options {
* A list of plugins for PostCSS,
* which are used before plugins loaded from PostCSS config file, if any
*/
plugins?: (postcss.Transformer | string | [string] | [string, object] | null | undefined)[];
plugins?: (
| postcss.Transformer
| string
| [string]
| [string, Record<string, unknown>]
| null
| undefined
)[];
/**
* Select mode for this plugin:
* - `"inject"` *(default)* - Embeds CSS inside JS and injects it into `<head>` at runtime.
Expand Down Expand Up @@ -238,7 +239,7 @@ export interface Options {
* Aliases for URL and import paths
* - ex.: `{"foo":"bar"}`
*/
alias?: { [from: string]: string };
alias?: Record<string, string>;
/**
* Enable and optionally pass additional configuration for
* [CSS Modules](https://github.com/css-modules/css-modules)
Expand Down
2 changes: 1 addition & 1 deletion src/utils/filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ export const isNullish = <T>(v: T): v is Extract<T, null | undefined> => {

export const nullishFilter = <T>(v: T): v is NonNullable<T> => !isNullish(v);

export const denullifyObject = <T>(o: { [x: string]: T }): { [x: string]: NonNullable<T> } =>
export const denullifyObject = <T>(o: Record<string, T>): Record<string, NonNullable<T>> =>
Object.entries(o)
.filter(([, v]) => nullishFilter(v))
.reduce((acc, [k, v]) => ({ ...acc, [k]: v }), {});
Expand Down
9 changes: 4 additions & 5 deletions src/utils/load-module.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import resolveAsync from "./resolve-async";

export interface ModuleImportMap {
export interface ModuleImportMap extends Record<string, unknown> {
sass: sass.Sass;
"node-sass": sass.Sass;
fibers: fibers.Fiber;
less: less.Less;
stylus: stylus.Stylus;
[x: string]: unknown;
}

export default async <T extends keyof ModuleImportMap>(
Expand All @@ -16,19 +15,19 @@ export default async <T extends keyof ModuleImportMap>(
if (typeof moduleId !== "string") return;

try {
return require(moduleId);
return require(moduleId) as ModuleImportMap[T];
} catch {
/* noop */
}

try {
return require(await resolveAsync(moduleId, { basedir }));
return require(await resolveAsync(moduleId, { basedir })) as ModuleImportMap[T];
} catch {
/* noop */
}

try {
return require(await resolveAsync(`./${moduleId}`, { basedir }));
return require(await resolveAsync(`./${moduleId}`, { basedir })) as ModuleImportMap[T];
} catch {
return;
}
Expand Down
6 changes: 3 additions & 3 deletions src/utils/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export function inferModeOption(mode: Options["mode"]): Mode {
return m;
}

export function inferHandlerOption<T extends { alias?: { [from: string]: string } }>(
export function inferHandlerOption<T extends { alias?: Record<string, string> }>(
option: T | boolean | undefined,
alias: T["alias"],
): T | false {
Expand All @@ -54,7 +54,7 @@ interface UseOpts {
stylus?: StylusLoaderOptions;
}
export function ensureUseOption(use: Options["use"], opts: UseOpts): LoadersOptions["use"] {
const all: { [x: string]: [string, object] } = {
const all: Record<string, [string, Record<string, unknown>]> = {
sass: ["sass", opts.sass ?? {}],
less: ["less", opts.less ?? {}],
stylus: ["stylus", opts.stylus ?? {}],
Expand All @@ -67,7 +67,7 @@ type PCSSOption = "parser" | "syntax" | "stringifier" | "plugin";
export function ensurePCSSOption<T>(option: T | string, type: PCSSOption): T {
if (typeof option !== "string") return option;
try {
return require(option);
return require(option) as T;
} catch {
throw new Error(`Unable to load PostCSS ${type} \`${option}\``);
}
Expand Down

0 comments on commit 4dd0ca3

Please sign in to comment.