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 support for type-checking .mdx files #225

Closed
wants to merge 2 commits into from

Conversation

phryneas
Copy link
Contributor

@phryneas phryneas commented Mar 8, 2019

This allows for typechecking .mdx files for use with systems like docz. (see #224 and doczjs/docz#664 )

To use it with docz, add the following to the doczrc.js:

import ForkTsCheckerPlugin from 'fork-ts-checker-webpack-plugin';

export default {
    modifyBundlerConfig: config => {
        config.plugins.push(
            new ForkTsCheckerPlugin({
                mdx: true,
                compilerOptions: {
                    noImplicitAny: false
                }
            })
        );

        return config;
    }
};

Note: as far as I am aware, this will not work for .mdx files if they are imported from another file as I did not transfer the resolveNonTsModuleName method from VueProgram.ts.
Apparently, it is also possible to use .mdx files to create components that are imported from normal TypeScript files.

This is something that would not be done in the context of docz which I am using - in docz, an mdx file is always a top level file. I don't know if anyone out there really uses it that way, so I did not implement it.

@johnnyreilly
Copy link
Member

Well done for putting something together that seems to meet your needs! I really appreciate the PR but I'm somewhat hesitant about adding direct support for .mdx files specifically.

I don't really want to keep expanding a switch statement and adding more and more program types to the plugin if possible as it creates more and more code to maintain.

I'd still like to help if it's feasible, but I wonder if we could think about creating a more "pluggable" approach, whereby people could supply their own Program to the plugin.

That way a thousand flowers could bloom without bloating the fork-ts-checker-webpack-plugin codebase.

What do you think?

@phryneas
Copy link
Contributor Author

phryneas commented Mar 8, 2019

I have to admit I kind of had the same thought, but if I had followed it to the end, that would also somehow question the existing vue support, which was nothing I wanted to suggest from the get-go, so I submitted it as-is for now and wanted to wait for your response.

I think something pluggable would be quite easy to realize.
fork-ts-checker-webpack-plugin could export an interface like this:

interface PluggableProgramFactory {
  createProgram(config: {
    typescript: typeof ts,
    configFile: string,
    compilerOptions: object
    files: FilesRegister,
    watcher: FilesWatcher,
    oldProgram: ts.Program
  }): { programConfig: ts.ParsedCommandLine; program: ts.Program};
  watchExtensions: string[];
}

that would need to be the top-level-export of any npm module to serve as a pluggable program.

The config could then just get the name of the npm module, pass it via environment variable to the service, be required there & the createProgram method could be invoked there (in place of loadDefaultProgram).

Is that in line with what you had in mind?

@johnnyreilly
Copy link
Member

johnnyreilly commented Mar 9, 2019

Is that in line with what you had in mind?

Pretty much - apart from this:

The config could then just get the name of the npm module, pass it via environment variable to the service, be required there

I'd have the pluggable program as an option that you pass to the plugin on initialisation. i.e. you'd do it in the webpack.config.js file.

Do you see what I mean? We already have the ability to supply custom formatters to the plugin - imagine this being the logical equivalent

@phryneas
Copy link
Contributor Author

phryneas commented Mar 9, 2019

I think I just wrote this bad - I wanted to say that it could be passed as a plugin constructor option.
All the rest was just because I wanted to highlight that it had to be a string, otherwise it wouldn't be able to be passed over the fork border (at least I don't see a way, maybe I'm missing something).
The formatters can be a function, but they never leave the main process so they don't have to be passed around, do they?

@johnnyreilly
Copy link
Member

Yup - you're right.

The new process / thread mechanism means that you're going to be working with unfortunate constraints.

Your mission (should you choose to accept it 😄 ) is to try and solve this as best you can. Experimentation encouraged!

@phryneas
Copy link
Contributor Author

phryneas commented Mar 9, 2019

Sounds like a fun quest. I'll give it a go in a separate PR ;)

@phryneas
Copy link
Contributor Author

phryneas commented Mar 9, 2019

well, see #227 ;)

@phryneas phryneas closed this Mar 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants