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

Added Node API #732

Merged
merged 9 commits into from
Nov 8, 2020
Merged

Added Node API #732

merged 9 commits into from
Nov 8, 2020

Conversation

JoshuaKGoldberg
Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg commented Oct 4, 2020

PR Checklist

Overview

Creates a new convertTSLintConfigStandalone method that's exported as convertTSLintConfig by src/index.ts, along with all the src/types helpers used by its public API. Its usage is explained in a new docs/API.md file:

  • It takes in the settings fields around config file locations and whether to use Prettier.
  • It outputs both a raw object and formatted string for the output ESLint configuration.

This method necessitated making the first half of the existing convertLintConfig available on their own, so it's split out into a new createESLintConfiguration method.

I also moved all the dependency initialization to its own src/api/dependencies.md file so it can be re-used in that standalone method.

@JoshuaKGoldberg JoshuaKGoldberg added the status: waiting for reviewer Waiting for a maintainer to review label Oct 4, 2020
@JoshuaKGoldberg JoshuaKGoldberg marked this pull request as ready for review October 4, 2020 03:51
@JoshuaKGoldberg
Copy link
Member Author

This should now be available on npm to try out locally via https://www.npmjs.com/package/tslint-to-eslint-config/v/2.0.0-beta0. 🚀

cc @JamesHenry

@JamesHenry
Copy link
Member

JamesHenry commented Oct 15, 2020

Thanks so much for publishing that @JoshuaKGoldberg!

After some digging I was able to get to the bottom of why my conversion was not behaving the same as when using the CLI (the rule severities and configs were not being reflected if you remember).

It is because full and raw are not equivalents, which is what I had been treating them as (in fact your code and docs in this PR currently also seems to).

Full is the result of running tslint --print-config and it is this JSON result which is what contains the necessary data about the rule severity and config which is then used when converting the config.

So I have managed to get it fully working after extracting some more parts of the library into my code.

I believe, however, I can avoid the need for that if you just expose to me the findReportedConfiguration() utility.

Here is an abridged version of my full usage of the library:

const reportedConfiguration = await findReportedConfiguration(
  childProcessExec,
  'npx tslint --print-config',
  pathToTslintJson
);

// ... error handling here...

const originalConfigurations = {
  tslint: {
    full: reportedConfiguration,
    raw: rawTslintJson,
  },
};

const summarizedConfiguration = await createESLintConfiguration(
  originalConfigurations
);

const convertedESLintConfig = joinConfigConversionResults(
  summarizedConfiguration,
  originalConfigurations
);

Let me know if you have any questions!

@JamesHenry
Copy link
Member

Thanks for the updates! Please could you publish another beta whenever you get chance? Not urgent!

@JoshuaKGoldberg
Copy link
Member Author

Super, published 2.0.0-beta1 @JamesHenry. 👍

@JamesHenry
Copy link
Member

This is looking really good! I haven't yet tackled the disable comment conversion logic, but for configs I think we are basically there. You can see my full usage of the API below for reference.

I have the following main request/callout as things stand:

With this usage below I am still ending up with an empty tslint-to-eslint-config.log being created on disk.

  • I can of course manually clean this up, but the way that schematics work is that the user will be able to see that that file existed and that it was deleted as part of the run (which is very confusing cause it was not in there when they ran the command) [see screenshot]

    • image
  • Please can we hunt down that createWriteStream and prevent it from being executed when using the Node API?


import type { Linter as ESLintLinter } from 'eslint';
import {
  createESLintConfiguration,
  findReportedConfiguration,
  joinConfigConversionResults,
} from 'tslint-to-eslint-config';

type TSLintRuleSeverity = 'warning' | 'error' | 'off';
type TSLintRuleOptions = {
  ruleArguments: any[];
  ruleName: string;
  ruleSeverity: TSLintRuleSeverity;
};

export async function convertToESLintConfig(
  pathToTslintJson: string,
  tslintJson: Record<string, unknown>,
): Promise<{
  convertedESLintConfig: ESLintLinter.Config;
  unconvertedTSLintRules: TSLintRuleOptions[];
}> {
  const reportedConfiguration = await findReportedConfiguration(
    'npx tslint --print-config',
    pathToTslintJson,
  );

  if (reportedConfiguration instanceof Error) {
    if (
      reportedConfiguration.message.includes('unknown option `--print-config')
    ) {
      throw new Error(
        '\nError: TSLint v5.18 required in order to run this schematic. Please update your version and try again.\n',
      );
    }
    throw new Error(`Unexpected error: ${reportedConfiguration.message}`);
  }

  const originalConfigurations = {
    tslint: {
      full: reportedConfiguration,
      raw: tslintJson,
    },
  };

  const summarizedConfiguration = await createESLintConfiguration(
    originalConfigurations,
  );

  const convertedESLintConfig = joinConfigConversionResults(
    summarizedConfiguration,
    originalConfigurations,
  ) as ESLintLinter.Config;

  return {
    convertedESLintConfig,
    unconvertedTSLintRules: summarizedConfiguration.missing,
  };
}

@JoshuaKGoldberg JoshuaKGoldberg added status: waiting for author The PR author should address requested changes and removed status: waiting for reviewer Waiting for a maintainer to review labels Nov 5, 2020
@JoshuaKGoldberg
Copy link
Member Author

Indeed, we can: the issue, I think, is that processLogger.ts creates a write stream immediately:

const debugFileName = "./tslint-to-eslint-config.log";

export const processLogger = {
    debugFileName,
    info: fs.createWriteStream(debugFileName),

...even though it's not used in this code. I'll switch it to only creating that write stream once, when first needed in calling .info().

@JoshuaKGoldberg
Copy link
Member Author

@JamesHenry try the latest beta2?

@JoshuaKGoldberg JoshuaKGoldberg added status: waiting for reviewer Waiting for a maintainer to review and removed status: waiting for author The PR author should address requested changes labels Nov 7, 2020
@JamesHenry
Copy link
Member

beta2 is looking good!

@JoshuaKGoldberg
Copy link
Member Author

Informal approvals from @KingDarBoja (over Gitter) and @JamesHenry (by virtue of #173 existing and me presuming), so I'm going to merge this! 🚀

If anybody has further requests on it, please do let me know somewhere and/or file an issue -- I'd like to tackle them before we stable release 2.0.0.

@JoshuaKGoldberg JoshuaKGoldberg merged commit 23dc93e into main Nov 8, 2020
@JoshuaKGoldberg JoshuaKGoldberg deleted the node-api branch November 8, 2020 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting for reviewer Waiting for a maintainer to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide node API for config conversion logic
2 participants