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

Add CLI #292

Open
4 tasks done
remcohaszing opened this issue Feb 2, 2023 · 14 comments
Open
4 tasks done

Add CLI #292

remcohaszing opened this issue Feb 2, 2023 · 14 comments
Labels
🤞 phase/open Post is being triaged manually

Comments

@remcohaszing
Copy link
Member

remcohaszing commented Feb 2, 2023

Initial checklist

Problem

In addition to type safety in editors, it would be nice if this can be validated in CI, similar to tsc.

Solution

Create a new package, @mdx-js/cli.

Some potential features of the CLI are:

  • Type checking
  • Emitting .d.ts and .js files for TypeScript, JavaScript, JSX, and MDX
  • Linting MDX using remark plugins similar to unified-engine

Alternatives

The idea is pretty vague still. There are many potential additional features and alternatives.

Related issues

@wooorm
Copy link
Member

wooorm commented Feb 2, 2023

this

What does this refer to?


Solution: you list several ideas. Which are most important?

Emitting .d.ts and .js files for TypeScript, JavaScript, JSX, and MDX

Why should this CLI also do things that TS does for .ts files? .js and .jsx files?

@remcohaszing
Copy link
Member Author

this

What does this refer to?

type safety

Solution: you list several ideas. Which are most important?

Type checking to begin with. I think most people use MDX with a framework to build a website. For such cases emitting files is not really important, that would be mostly for libraries.

Emitting .d.ts and .js files for TypeScript, JavaScript, JSX, and MDX

Why should this CLI also do things that TS does for .ts files? .js and .jsx files?

The language service already needs all of these files to get a full context of the project. I don’t think it will take any extra effort to emit the JavaScript files. In fact, flags that clean the output directory might get in the way when using both tsc and the MDX CLI.

@adueck
Copy link

adueck commented Feb 4, 2023

YES!! This is a dream come true to see these discussions happening. I have a project with a bunch of .mdx files with complex data imports etc, and for a long time I have wished there was a pipeline for type checking these files. This kind of thing will let mdx fly as a first class citizen in the growing world of TypeScript development! 🚀

@ChristianMurphy
Copy link
Member

Potentially related to #298, Volar is working on a Scripts API:

The Scripts API is designed to expose the formatting and linting capabilities of the language server so that they can be used in scripts, allowing you to use it in CI or git pre-commit hooks and get the same results as you would in an IDE.

https://blog.vuejs.org/posts/volar-a-new-beginning.html

@wooorm
Copy link
Member

wooorm commented Feb 28, 2023

For such cases emitting files is not really important, that would be mostly for libraries.

Do you know of an example of a library that has several MDX files that it exposes?

Type checking to begin with.

Couldn’t TypeScript perform type checking? What’s the reason a different program is needed to do that?

I don’t think it will take any extra effort to emit the JavaScript files. In fact, flags that clean the output directory might get in the way when using both tsc and the MDX CLI.

Maybe it isn’t a lot of work to implement your ideas, but I would also like to hear a benefit for why code should exists?
As you mention: two tools doing the same, will cause conflicts.

Create a new package, @mdx-js/cli.

Some potential features of the CLI are:

What is the reason for one CLI (@mdx-js/cli) to exist for varying needs, instead of a CLI that specifically focussed on the primary problem at hand, the types(cript) part? @mdx-js/tsc or so? @mdx-js/analyzer?

@remcohaszing
Copy link
Member Author

For such cases emitting files is not really important, that would be mostly for libraries.

Do you know of an example of a library that has several MDX files that it exposes?

No.

Type checking to begin with.

Couldn’t TypeScript perform type checking? What’s the reason a different program is needed to do that?

TypeScript can’t handle MDX, not for editor features, nor type checking.

I don’t think it will take any extra effort to emit the JavaScript files. In fact, flags that clean the output directory might get in the way when using both tsc and the MDX CLI.

Maybe it isn’t a lot of work to implement your ideas, but I would also like to hear a benefit for why code should exists? As you mention: two tools doing the same, will cause conflicts.

I don’t consider this doing two things. Type checking and emitting both fall in the category of making a CLI like tsc that understands MDX.

Create a new package, @mdx-js/cli.
Some potential features of the CLI are:

What is the reason for one CLI (@mdx-js/cli) to exist for varying needs, instead of a CLI that specifically focussed on the primary problem at hand, the types(cript) part? @mdx-js/tsc or so? @mdx-js/analyzer?

I consider the first two points to be more or less the same thing, with slightly different options.

The last point is more interesting. Currently we have MDX and remark-cli / remark-language-server. Both are powerful and use the same underlying technology. Combining them should be straight-forward. I have no strong opinion on what this should look like, but it’s probably one of:

  • Create one MDX CLI with unified-engine support and type checking support.
  • Add MDX support to remark-cli.
  • Create another CLI and language server for MDX based on unified-engine.

I think this issue will become redundant and we should hook into Volar at some point to get all their TypeScript related features, including a CLI, without having to maintain this separately.

@wooorm
Copy link
Member

wooorm commented Mar 3, 2023

TypeScript can’t handle MDX, not for editor features, nor type checking.

I don’t think I understand you at all, because as far as I understand, TypeScript can check types if someone authors types.

You yourself have documented that it works: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/b92231c3ef87e84860ea674ec38d8e694e5dc64e/types/mdx/index.d.ts#L47-L53.

@karlhorky
Copy link

karlhorky commented Jul 23, 2024

I'm thinking through trying to use the MDX TypeScript plugin package on the command line / terminal 🤔

Do you think that some (non-officially supported) MVP of checking types of syntax and usage in *.mdx files could be put together with something like ts-patch?

(before official TypeScript support for *.mdx, if that ever happens)

Maybe ts-patch could be used as such:

tsconfig.json

{
  "compilerOptions": {
    "plugins": [
      {
        "name": "./path/to/mdx-analyzer/packages/typescript-plugin"
      }
    ]
  },
  "include": ["src/**/*.ts"],
  "exclude": ["node_modules"]
}

Usage with tspc binary from ts-patch ("The live compiler patches on-the-fly, each time it is run"):

npx tspc --project path/to/your/tsconfig.json

@karlhorky
Copy link

karlhorky commented Jul 23, 2024

I don’t think I understand you at all, because as far as I understand, TypeScript can check types if someone authors types.

You yourself have documented that it works: DefinitelyTyped/DefinitelyTyped@b92231c/types/mdx/index.d.ts#L47-L53.

I think this is just for the public interface, not checking of syntax in the .mdx file.

I'm guessing aside from checking the public interface, most users also want checking types of syntax in the .mdx file, like:

  1. props with incorrect types passed to components in the .mdx file
  2. invalid imports in the .mdx file
  3. checking of JSDoc types in the .mdx file

@ChristianMurphy
Copy link
Member

I'll defer more to @remcohaszing who has more expertise in this particular space.

Do you think that some (non-officially supported) MVP of checking types of syntax and usage in *.mdx files could be put together with something like ts-patch?

Using an unofficial compiler integration is something I'd be a bit cautious about.
My experience from other similar unofficial integrations, they're brittle and a moving target.
I'm not opposed if anyone in the community has time and wants to.
I'd feel a lot more comfortable if we could use an official API from Volar or TypeScript instead of something more bespoke.

@remcohaszing
Copy link
Member Author

I haven’t tried ts-patch. you can give it a try. At the moment Astro and Vue provide their own CLI for this. IMO this is suboptimal, because MDX is compatible with frameworks like Astro and Vue as well.

@karlhorky
Copy link

karlhorky commented Sep 29, 2024

At the moment Astro and Vue provide their own CLI for this

Oh wow, the code from the vue-tsc CLI (linked in Vue.js docs) is not large! ❤️ Probably in large part thanks to Volar.

vuejs/language-tools/blob/master/packages/tsc/index.ts

import { runTsc } from '@volar/typescript/lib/quickstart/runTsc';
import * as vue from '@vue/language-core';

const windowsPathReg = /\\/g;

export function run(tscPath = require.resolve('typescript/lib/tsc')) {

	let runExtensions = ['.vue'];

	const extensionsChangedException = new Error('extensions changed');
	const main = () => runTsc(
		tscPath,
		runExtensions,
		(ts, options) => {
			const { configFilePath } = options.options;
			const vueOptions = typeof configFilePath === 'string'
				? vue.createParsedCommandLine(ts, ts.sys, configFilePath.replace(windowsPathReg, '/')).vueOptions
				: vue.resolveVueCompilerOptions({});
			const allExtensions = vue.getAllExtensions(vueOptions);
			if (
				runExtensions.length === allExtensions.length
				&& runExtensions.every(ext => allExtensions.includes(ext))
			) {
				const vueLanguagePlugin = vue.createVueLanguagePlugin<string>(
					ts,
					options.options,
					vueOptions,
					id => id
				);
				return { languagePlugins: [vueLanguagePlugin] };
			}
			else {
				runExtensions = allExtensions;
				throw extensionsChangedException;
			}
		}
	);

	try {
		main();
	} catch (err) {
		if (err === extensionsChangedException) {
			main();
		} else {
			throw err;
		}
	}
}

And doesn't use any 3rd-party, unofficial compiler patcher 👍

So maybe there's a way to get to an MVP of some simple CLI like this from the MDX TypeScript plugin package... 🤔

Maybe a better option for me to check out than the ts-patch option above.

@karlhorky
Copy link

karlhorky commented Sep 29, 2024

Hacked together a very messy (and probably wrong in many ways) first version that is showing TypeScript errors from .mdx files on the command line! 🙌 🎉

$ node mdx-tsc.js
1.mdx:3:16 - error TS2339: Property 'bbbb' does not exist on type '{ readonly a: 1; readonly components?: {} | undefined; }'.

3 <div id={props.bbbb}>f</div>

1.mdx

{/** @typedef {{ a: 1 }} Props */}

<div id={props.bbbb}>f</div>

mdx-tsc.js

#!/usr/bin/env node

// ```
// npm install @volar/typescript @mdx-js/language-service load-plugin remark-frontmatter remark-gfm
// ```

import * as path from 'node:path';
import { fileURLToPath, pathToFileURL } from 'node:url';
import {
  createMdxLanguagePlugin,
  resolveRemarkPlugins,
} from '@mdx-js/language-service';
import { runTsc } from '@volar/typescript/lib/quickstart/runTsc.js';
import { loadPlugin } from 'load-plugin';
import remarkFrontmatter from 'remark-frontmatter';
import remarkGfm from 'remark-gfm';
import ts from 'typescript';

function getTsconfigPathFromArgs() {
  const args = process.argv.slice(2); // Skip 'node' and script path
  for (let i = 0; i < args.length; i++) {
    const arg = args[i];
    if (arg === '--project' || arg === '-p') {
      const nextArg = args[i + 1];
      if (nextArg && !nextArg.startsWith('-')) {
        return nextArg;
      }
    } else if (arg.startsWith('--project=')) {
      return arg.split('=')[1];
    } else if (arg.startsWith('-p')) {
      if (arg.length > 2) {
        // e.g., -pmyconfig.json
        return arg.slice(2);
      } else {
        // Next argument is the value
        const nextArg = args[i + 1];
        if (nextArg && !nextArg.startsWith('-')) {
          return nextArg;
        }
      }
    }
  }
  return 'tsconfig.json'; // Default path
}

async function run() {
  // Use import.meta.resolve to get the URL to 'typescript/lib/tsc.js'
  const tscUrl = import.meta.resolve('typescript/lib/tsc.js');
  const tscPath = fileURLToPath(tscUrl);

  // Parse command-line arguments and tsconfig.json
  const tsconfigPath = getTsconfigPathFromArgs();
  const args = process.argv.slice(2);

  let mdxOptions = {};
  let cwd = process.cwd();
  let parsedConfig;
  let compilerOptions;

  if (typeof tsconfigPath === 'string') {
    const configResult = ts.readConfigFile(tsconfigPath, ts.sys.readFile);
    if (configResult.error) {
      throw new Error(`Error reading tsconfig.json at ${tsconfigPath}`);
    }
    const configFile = configResult.config;
    parsedConfig = ts.parseJsonConfigFileContent(
      configFile,
      ts.sys,
      path.dirname(tsconfigPath),
    );
    mdxOptions = parsedConfig.raw?.mdx || {};
    cwd = path.dirname(tsconfigPath);
  }

  // Parse compiler options from command-line arguments
  const commandLine = ts.parseCommandLine(args);
  compilerOptions = { ...parsedConfig.options, ...commandLine.options };

  // Resolve Remark plugins asynchronously
  let plugins;
  try {
    plugins = await resolveRemarkPlugins(mdxOptions, (name) =>
      loadPlugin(name, { prefix: 'remark', from: pathToFileURL(cwd).href }),
    );
  } catch (e) {
    // Fallback to default plugins if resolving fails
    plugins = [[remarkFrontmatter, ['toml', 'yaml']], remarkGfm];
  }

  const jsxImportSource = compilerOptions.jsxImportSource;

  // Create MDX language plugin with resolved plugins and options
  const mdxLanguagePlugin = createMdxLanguagePlugin(
    plugins,
    Boolean(mdxOptions?.checkMdx),
    jsxImportSource,
  );

  const languagePlugins = [mdxLanguagePlugin];

  runTsc(tscPath, ['.mdx'], (ts, options) => {
    // Synchronous callback using pre-resolved language plugins
    return { languagePlugins };
  });
}

run();

I'll try to clean this up and make it more correct.

@remcohaszing
Copy link
Member Author

PR welcome, even if it’s just a PoC. The fixtures directory contains some interesting fixtures you may want to use for (manual or integration) testing, or you could add your own fixtures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤞 phase/open Post is being triaged manually
Development

No branches or pull requests

5 participants