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

Mix esbuild-loader and ts-loader #915

Closed
thomaschaaf opened this issue Mar 4, 2021 · 12 comments
Closed

Mix esbuild-loader and ts-loader #915

thomaschaaf opened this issue Mar 4, 2021 · 12 comments

Comments

@thomaschaaf
Copy link

I would like to use emitDecoratorMetadata but it is not supported by esbuild would it be possible to use ts-loader only for files that use decorators?

@evanw
Copy link
Owner

evanw commented Mar 4, 2021

Yes, I think that should be fairly straightforward. You could try writing an on-load plugin that uses a regular expression to detect whether there is probably a decorator in the file or not. If there is, you could call out to the TypeScript compiler and return the JavaScript to esbuild instead of letting esbuild convert the TypeScript to JavaScript for those files.

@evanw
Copy link
Owner

evanw commented Mar 5, 2021

Since you mentioned esbuild-loader and ts-loader, I assume you're using Webpack. I'm not familiar with writing Webpack plugins at all so I'm not going to be able to help too much. Doing this is possible with an esbuild plugin but it may be that the right way to do this is to write a Webpack plugin instead that "composes" esbuild-loader and ts-loader (i.e. decides which one to run based on the regular expression and the file contents). Or perhaps the authors of esbuild-loader and/or ts-loader might be interested in integrating something like this into their plugin? I'm not sure what the right approach is because I don't know anything about Webpack myself.

I'm going to close this issue since it's not an issue with esbuild itself, and this is not something esbuild itself can fix.

@evanw evanw closed this as completed Mar 5, 2021
@thomaschaaf
Copy link
Author

Thank you for your extensive reply! I looked into the plugins but sadly saw that they are not currently available for the transform api and esbuild-loader is using that. I am now using webpacks loaders to achieve this: https://github.com/privatenumber/esbuild-loader/discussions/117

I am interested to see what speeds I can achieve with esbuild without webpack and will try to get something running by using the Typescript Compiler API if I notice a decorator present.
https://github.com/microsoft/TypeScript-wiki/blob/master/Using-the-Compiler-API.md#a-simple-transform-function

@thomaschaaf
Copy link
Author

@evanw it's very hard for me to detect if a file contains a decorator. Thus I'm thinking of compiling the code with esbuild and then check for __decorate in the esbuild output and compile it again with tsc. Is there a hook available for this? Or could you create a new one where I could do this?

@evanw
Copy link
Owner

evanw commented Mar 6, 2021

it's very hard for me to detect if a file contains a decorator

I was imagining you could do something like /@\w+/.test(contentsOfFile) to reject files that definitely don't contain a decorator. That might look something like this (a real-world plugin would likely read from tsconfig.json somehow):

let tscDecoratorPlugin = {
  name: 'tsc-decorator',
  setup(build) {
    let options = {
      // Note: This would be more complicated in practice
      experimentalDecorators: true,
      emitDecoratorMetadata: true,
    };

    // Note: May want to support "*.tsx" too
    build.onLoad({ filter: /\.ts$/ }, async (args) => {
      let ts = await require('fs').promises.readFile(args.path, 'utf8')
      if (/@\w+/.test(ts)) {
        let program = require('typescript').createProgram([args.path], options);
        let contents;
        program.emit(void 0, (path, js) => contents = js);
        return { contents };
      }
    })
  },
}

require('esbuild').build({
  entryPoints: ['entry.ts'],
  bundle: true,
  outfile: 'out.js',
  plugins: [tscDecoratorPlugin],
}).catch(() => process.exit(1))

@thomaschaaf
Copy link
Author

thomaschaaf commented Mar 6, 2021

Just checking for an @ is too naive. Npm repos often use orgs -> require('@org/reponame') and people could use @ in comments like jsdoc or just copyright statements with email addresses. Thus this would limit the speed increase too much.

@evanw
Copy link
Owner

evanw commented Mar 6, 2021

Well in that case checking for __decorate should work I think. You could try something like this:

// First pass: is there "@word" without being at the start of a string?
let probablyHasDecorator = /(?<!['"])@\w+/.test(tsCode)
if (probablyHasDecorator) {
  // Second pass: is it likely that esbuild detected a decorator?
  let jsCode = await esbuild.transform(tsCode, { loader: 'ts' })
  probablyHasDecorator = jsCode.includes('__decorate')
}

@thomaschaaf
Copy link
Author

thomaschaaf commented Mar 6, 2021

Edit:
Sorry I didn't see your reply. Same question with the esbuild.transform output. How can I not loose sourcemaps?

@evanw I have enhanced your example here. How should I handle the sourcemaps? Should I override the sourcemaps option in tsc and use inlineSourceMap?

const esbuild = require('esbuild');
const fs = require('fs').promises;
const typescript = require('typescript');
const tsconfigPath = './tsconfig.json';
const stripComments = require('strip-comments');

const tscDecoratorPlugin = {
  name: 'tsc-decorator',
  setup(build) {
    let options = null;

    const hasDecorator = (fileContent, offset = 0) => {
      let cleanedFileContents = fileContent;

      if (offset === 0) {
        // check if file contains an @ at all
        if (fileContent.indexOf('@', offset) === -1) {
          return false;
        }

        // strip comments which could contain @
        cleanedFileContents = stripComments(fileContent);
      }

      const atPosition = cleanedFileContents.indexOf('@', offset);

      if (atPosition === -1) {
        return false;
      }

      if (atPosition === 0) {
        return true;
      }

      // ignore "@ and '@ as they are used in require('@org/repo')
      if (["'", '"'].includes(cleanedFileContents.substr(atPosition - 1, 1))) {
        return hasDecorator(cleanedFileContents, atPosition + 1);
      }

      return true;
    };

    build.onLoad({ filter: /\.ts$/ }, async (args) => {
      if (!options) {
        options = JSON.parse(
          stripComments(await fs.readFile(tsconfigPath, 'utf8')) || null
        );
      }

      const ts = await fs.readFile(args.path, 'utf8');
      if (
        options.compilerOptions &&
        options.compilerOptions.emitDecoratorMetadata &&
        hasDecorator(ts)
      ) {
        console.log('compiling with tsc', args.path);
        const program = typescript.transpileModule(ts, options);
        // how can I pass the sourcemap to eslint?
        // program.sourceMapText
        return { contents: program.outputText };
      } else {
        console.log('compiling with esbuild', args.path);
      }
    });
  },
};

esbuild
  .build({
    entryPoints: [
      'handler.ts'
    ],
    bundle: true,
    plugins: [tscDecoratorPlugin],
  })
  .catch(() => process.exit(1));

@evanw
Copy link
Owner

evanw commented Mar 6, 2021

I have enhanced your example here.

Looks good to me. Searching through the string like this should be the fastest option by far.

How should I handle the sourcemaps? Should I override the sourcemaps option in tsc and use inlineSourceMap?

Yes that's right. I would also enable inlineSources.

@thomaschaaf
Copy link
Author

thomaschaaf commented Mar 6, 2021

I have done some testing on a project of mine and actually having some false positives is faster than having to pipe everything through javascript with

let jsCode = await esbuild.transform(tsCode, { loader: 'ts' })

I released the plugin here: https://github.com/thomaschaaf/esbuild-plugin-tsc

Thank you for your help!

@Djaler
Copy link
Contributor

Djaler commented Mar 27, 2021

@thomaschaaf but how you can use this plugin in webpack build?

@thomaschaaf
Copy link
Author

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

No branches or pull requests

3 participants