Skip to content

Commit

Permalink
refactor: invert some conditionals for better readability (#335)
Browse files Browse the repository at this point in the history 
- 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
  • Loading branch information
@agilgur5
agilgur5 committed Jun 1, 2022
1 parent e145d0b commit 01272d3
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 70 deletions.
130 changes: 61 additions & 69 deletions src/index.ts
View file
Expand Up @@ -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)
Expand Down Expand Up @@ -229,39 +227,37 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
printDiagnostics(contextWrapper, diagnostics, parsedConfig.options.pretty === true);
}

if (result)
{
if (result.references)
result.references.map(normalize).map(allImportedFiles.add, allImportedFiles);
if (!result)
return undefined;

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.references)
result.references.map(normalize).map(allImportedFiles.add, allImportedFiles);

if (result.dts)
{
const key = normalize(id);
declarations[key] = { type: result.dts, map: result.dtsmap };
context.debug(() => `${blue("generated declarations")} for '${key}'`);
}
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: ")}`);
}

const transformResult: TransformResult = { code: result.code, map: { mappings: "" } };
if (result.dts)
{
const key = normalize(id);
declarations[key] = { type: result.dts, map: result.dtsmap };
context.debug(() => `${blue("generated declarations")} for '${key}'`);
}

if (result.map)
{
if (pluginOptions.sourceMapCallback)
pluginOptions.sourceMapCallback(id, result.map);
transformResult.map = JSON.parse(result.map);
}
const transformResult: TransformResult = { code: result.code, map: { mappings: "" } };

return transformResult;
if (result.map)
{
if (pluginOptions.sourceMapCallback)
pluginOptions.sourceMapCallback(id, result.map);
transformResult.map = JSON.parse(result.map);
}

return undefined;
return transformResult;
},

generateBundle(bundleOptions)
Expand Down Expand Up @@ -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) =>
Expand Down
2 changes: 1 addition & 1 deletion src/tscache.ts
View file
Expand Up @@ -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;
}));
}

Expand Down

0 comments on commit 01272d3

Please sign in to comment.