Skip to content

Start language service integration. #118

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

Closed
wants to merge 2 commits into from

Conversation

strothj
Copy link
Contributor

@strothj strothj commented Aug 29, 2018

Hello,

Please don't merge this. This is a work in progress. I just want to see if this would be something you'd be likely to merge in and to get some initial feedback.

What I'd like to do is integrate the TypeScript language service into this package. This solves #112.

I was encountering poor performance on projects that hit large numbers of components (80+). I've reused the code from ts-loader (https://github.com/TypeStrong/ts-loader/blob/master/src/servicesHost.ts) to create a reusable instance of ts.Program. On one especially large project, I saw a drop from multiple minutes to about 30 seconds.

Issues needing resolving:

  • Determine how we want to detect the user's project root. This is one of the following in ts-loader: webpack's context setting, location of tsconfig.json, current working directory.
  • Possibily add a setting to explicitly enable this behavior
  • Changes to files in the project are not detected. We will need to fill in the watcher setting in the language service setup. This was previously done by wiring it into the Webpack system. Currently, things compile as expecedt but changes are not reflected in watch mode.
  • Bring over relevant unit tests

We can select which behavior to use (file by file or language service) by requesting a service host instance or creating a ts.Program.
parser.ts:

      const { instance } = getTypeScriptInstance(
        process.cwd(),
        compilerOptions
      );

      const filePaths = Array.isArray(filePathOrPaths)
        ? filePathOrPaths
        : [filePathOrPaths];

      const program = instance.languageService!.getProgram()!;
      // const program = ts.createProgram(filePaths, compilerOptions);

@tutman96
Copy link

Nice work! I just cloned your branch and copy-pasted your version into my application and it works great! Nice and fast. The watching did work for me however... so I don't know what your issue was.

@strothj
Copy link
Contributor Author

strothj commented Aug 29, 2018

I think the issue I had with watching was from testing in Storybook. This package is used by another as a Webpack loader.

I think I will need to hook into the file changed event from the watcher. I’d then need to supply an async version of this package’s export so that the loader can postpone the update until the watcher has finished processing.

@strothj
Copy link
Contributor Author

strothj commented Aug 29, 2018

I found it easier to test by using the utility called yalc. You type yalc publish in this package directory and then type yalc link react-docgen-typescript in your project.

@tutman96
Copy link

Gotcha.... I needed to get it to the rest of my team though, so I just pasted it into our repo :D

Thanks again for this awesome work! The team really loves the improvements

@strothj
Copy link
Contributor Author

strothj commented Aug 31, 2018

I've decided to take a slightly different approach to this problem. Submitting another pull request with a more digestible change.

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