refactor: invert some conditionals for better readability (#335)

- a few `if (cond) { big block } return` could be inverted to
  `if (!cond) return` then the block unindented instead
  - generally speaking, this makes it a lot more readable (less
    indentation, etc) and follows the existing code style in much of the
    codebase, where it's a few quick one-line ifs and then just the rest
    of the code

- shorten the resolvedFileName conditional by using optional chaining
  - prefer the simpler `x?.y` over the older `x && x.y` syntax
- add a `resolved` variable for less repetition of the whole statement
- add a comment to the `pathNormalize` line about why it's used in that
  one place and link to the longer description in the PR as well

- shorten comment about `useTsconfigDeclarationDir` so it doesn't take
  up so much space or look so important as a result
  - and it's easier to read this way and I made the explanation less
    verbose and quicker to read too
- remove the `else` there and just add an early return instead, similar
  to the inverted conditionals above
  - similarly makes it less unindented, more readable etc
This commit is contained in:
Anton Gilgur 2022-05-31 22:42:36 -04:00 committed by GitHub
parent e145d0baeb
commit 01272d3ebe
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 66 additions and 74 deletions

View File

@ -154,26 +154,24 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
// TODO: use module resolution cache
const result = tsModule.nodeModuleNameResolver(importee, importer, parsedConfig.options, tsModule.sys);
let resolved = result.resolvedModule?.resolvedFileName;
if (result.resolvedModule && result.resolvedModule.resolvedFileName)
{
if (filter(result.resolvedModule.resolvedFileName))
cache().setDependency(result.resolvedModule.resolvedFileName, importer);
if (!resolved)
return;
if (result.resolvedModule.resolvedFileName.endsWith(".d.ts"))
return;
if (filter(resolved))
cache().setDependency(resolved, importer);
const resolved = pluginOptions.rollupCommonJSResolveHack
? resolve.sync(result.resolvedModule.resolvedFileName)
: result.resolvedModule.resolvedFileName;
if (resolved.endsWith(".d.ts"))
return;
context.debug(() => `${blue("resolving")} '${importee}' imported by '${importer}'`);
context.debug(() => ` to '${resolved}'`);
if (pluginOptions.rollupCommonJSResolveHack)
resolved = resolve.sync(resolved);
return pathNormalize(resolved);
}
context.debug(() => `${blue("resolving")} '${importee}' imported by '${importer}'`);
context.debug(() => ` to '${resolved}'`);
return;
return pathNormalize(resolved); // use host OS separators to fix Windows issue: https://github.com/ezolenko/rollup-plugin-typescript2/pull/251
},
load(id)
@ -229,39 +227,37 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
printDiagnostics(contextWrapper, diagnostics, parsedConfig.options.pretty === true);
}
if (result)
if (!result)
return undefined;
if (result.references)
result.references.map(normalize).map(allImportedFiles.add, allImportedFiles);
if (watchMode && this.addWatchFile && result.references)
{
if (result.references)
result.references.map(normalize).map(allImportedFiles.add, allImportedFiles);
if (watchMode && this.addWatchFile && result.references)
{
if (tsConfigPath)
this.addWatchFile(tsConfigPath);
result.references.map(this.addWatchFile, this);
context.debug(() => `${green(" watching")}: ${result.references!.join("\nrpt2: ")}`);
}
if (result.dts)
{
const key = normalize(id);
declarations[key] = { type: result.dts, map: result.dtsmap };
context.debug(() => `${blue("generated declarations")} for '${key}'`);
}
const transformResult: TransformResult = { code: result.code, map: { mappings: "" } };
if (result.map)
{
if (pluginOptions.sourceMapCallback)
pluginOptions.sourceMapCallback(id, result.map);
transformResult.map = JSON.parse(result.map);
}
return transformResult;
if (tsConfigPath)
this.addWatchFile(tsConfigPath);
result.references.map(this.addWatchFile, this);
context.debug(() => `${green(" watching")}: ${result.references!.join("\nrpt2: ")}`);
}
return undefined;
if (result.dts)
{
const key = normalize(id);
declarations[key] = { type: result.dts, map: result.dtsmap };
context.debug(() => `${blue("generated declarations")} for '${key}'`);
}
const transformResult: TransformResult = { code: result.code, map: { mappings: "" } };
if (result.map)
{
if (pluginOptions.sourceMapCallback)
pluginOptions.sourceMapCallback(id, result.map);
transformResult.map = JSON.parse(result.map);
}
return transformResult;
},
generateBundle(bundleOptions)
@ -330,44 +326,40 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
if (fileName.includes("?")) // HACK for rollup-plugin-vue, it creates virtual modules in form 'file.vue?rollup-plugin-vue=script.ts'
fileName = fileName.split(".vue?", 1) + extension;
// If 'useTsconfigDeclarationDir' is given in the
// plugin options, directly write to the path provided
// by Typescript's LanguageService (which may not be
// under Rollup's output directory, and thus can't be
// emitted as an asset).
// If 'useTsconfigDeclarationDir' is in plugin options, directly write to 'declarationDir'.
// This may not be under Rollup's output directory, and thus can't be emitted as an asset.
if (pluginOptions.useTsconfigDeclarationDir)
{
context.debug(() => `${blue("emitting declarations")} for '${key}' to '${fileName}'`);
tsModule.sys.writeFile(fileName, entry.text, entry.writeByteOrderMark);
return;
}
else
// don't mutate the entry because generateBundle gets called multiple times
let entryText = entry.text
const declarationDir = (_output.file ? dirname(_output.file) : _output.dir) as string;
const cachePlaceholder = `${pluginOptions.cacheRoot}/placeholder`
// modify declaration map sources to correct relative path
if (extension === ".d.ts.map")
{
// don't mutate the entry because generateBundle gets called multiple times
let entryText = entry.text
const declarationDir = (_output.file ? dirname(_output.file) : _output.dir) as string;
const cachePlaceholder = `${pluginOptions.cacheRoot}/placeholder`
// modify declaration map sources to correct relative path
if (extension === ".d.ts.map")
const parsedText = JSON.parse(entryText) as SourceMap;
// invert back to absolute, then make relative to declarationDir
parsedText.sources = parsedText.sources.map(source =>
{
const parsedText = JSON.parse(entryText) as SourceMap;
// invert back to absolute, then make relative to declarationDir
parsedText.sources = parsedText.sources.map(source =>
{
const absolutePath = pathResolve(cachePlaceholder, source);
return normalize(relative(declarationDir, absolutePath));
});
entryText = JSON.stringify(parsedText);
}
const relativePath = normalize(relative(cachePlaceholder, fileName));
context.debug(() => `${blue("emitting declarations")} for '${key}' to '${relativePath}'`);
this.emitFile({
type: "asset",
source: entryText,
fileName: relativePath,
const absolutePath = pathResolve(cachePlaceholder, source);
return normalize(relative(declarationDir, absolutePath));
});
entryText = JSON.stringify(parsedText);
}
const relativePath = normalize(relative(cachePlaceholder, fileName));
context.debug(() => `${blue("emitting declarations")} for '${key}' to '${relativePath}'`);
this.emitFile({
type: "asset",
source: entryText,
fileName: relativePath,
});
};
Object.keys(declarations).forEach((key) =>

View File

@ -70,7 +70,7 @@ export function getAllReferences(importer: string, snapshot: tsTypes.IScriptSnap
return _.compact(info.referencedFiles.concat(info.importedFiles).map((reference) =>
{
const resolved = tsModule.nodeModuleNameResolver(reference.fileName, importer, options, tsModule.sys);
return resolved.resolvedModule ? resolved.resolvedModule.resolvedFileName : undefined;
return resolved.resolvedModule?.resolvedFileName;
}));
}