Loadable-components: TypeScript custom transformer for SSR

[Quramy]

🚀 Feature Proposal

Hi. Thanks for this great tool.

Do you have any plan to provide a TypeScript custom transformer such as @loadable/babel-plugin ?

Motivation

I'm working with a project using React/SSR/TypeScript. So we use not babel-loader but ts-loader with webpack. We're considering to introduce loadable-components but we don't want install Babel tools.

Example

/* webpack.config.js */
const loadableComponentsTransformer = require('@loadable/ts-transformer');

module.exports = {
    ...
    module: {
      rules: [
        {
          test: /\.tsx?$/,
          exclude: /node_modules/,
          loader: "ts-loader",
          options: {
            getCustomTransformers: () => ({ before: [loadableComponentsTransformer] }),
          },
        },
      ],
    },
    ...
};

See also https://github.com/TypeStrong/ts-loader#getcustomtransformers--program-program---before-transformerfactory-after-transformerfactory-- .

Pitch

Why does this feature belong in the Loadable Component ecosystem?

[theKashey]

As long as almost nobody provides, or even know about ts-transformers - I personally always use ts-loader + babel, and many others just use babel to transpile TS.
Adding this, not high demand feature, duplicating another piece of functionality, would create one more maintenance burden. Is it worth it?

[codesandtags]

@Quramy Were you able to start the server using this configuration? I've started a project which uses react + typescript + loadable/components but I'm not able to start correctly the server.

[codesandtags]

I think this is a good reference to achieve this goal.

https://medium.com/@minoo/react-typescript-ssr-code-splitting-%ED%99%98%EA%B2%BD%EC%84%A4%EC%A0%95%ED%95%98%EA%B8%B0-d8cec9567871

[joe06102]

it is a great feature for someone like me doing all stuff with only tsc!

[Quramy]

I wrote it by myself.
https://github.com/Quramy/loadable-ts-transformer

[joe06102]

I wrote it by myself.
https://github.com/Quramy/loadable-ts-transformer

Thx a lot.It's great. BTW, I wanna know how can I learn to write the transformer function? The ts-loader doc does not provide much info.

[Quramy]

@joe06102
I’ve learned ts transformers API by reading ts original sources.
For example, https://github.com/microsoft/TypeScript/blob/master/src/compiler/transformers/es2016.ts helped me so much.

[joe06102]

@joe06102
I’ve learned ts transformers API by reading ts original sources.
For example, https://github.com/microsoft/TypeScript/blob/master/src/compiler/transformers/es2016.ts helped me so much.

Thx, :D

[theKashey]

🙀🚀🤯🥳👍

[theKashey]

It would be great to move transformer here, and ever greater to share tests/expectations between them.

[dagda1]

@theKashey @Quramy this is absolutely amazing, finally, I don't have to use babel. Works like a treat for me.

If there is anything I can do to help get this into the source then please let me know.

great work done by @Quramy

[dagda1]

@theKashey @Quramy I just blogged about this here for logrocket

[theKashey]

@dagda1 - there is no link :(

[dagda1]

@theKashey ooops here it is

[dagda1]

really love to get this into the loadable-components. is there anything i can do?

[theKashey]

Open a PR adding packages/ts-transformer into the loadable. Plus the hard part - documentation.

[dagda1]

@theKashey happy to have a go at this. So I should create a new typescript section in both the guides and the api section of the gatsby website?

[theKashey]

I think yes. And don't forget that using ts+babel, or just babel-ts would be still prefered way for many of us.

[dagda1]

@theKashey I am surprised to hear that. Why is babel-ts the preferred way? I am just curious as to why people would prefer that. My thoughts are that one transpiler is enough

[theKashey]

It's just a bit different typescript - like isolatedModules everywhere. And, as long as people __have__ to use babel for other stuff, for example we need at least js-lingui, they tend to keep only one "compiling tool".
ts-transformers are quite new, there were no options just yesterday.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[theKashey]

Not today @stalebot

[dagda1]

@theKashey my bad, I totally forgot about this. I will commit to doing this if you are still open to a PR. I am using it daily and this is the first time I have code-splitting so would love to add it.

[theKashey]

Well, actually we should team up - there are a few issues with babel plugin right now (#568), and some changes have to be synchronized if you want to get a fix as well.

[dagda1]

Absolutely, would love to. This is a great package and I would love to give back

[dagda1]

@theKashey do you mean you want #568 green and ready to merge?

[theKashey]

Not green yet, and will need a few tweaks, but isReady has to to be changed in any case. And look like #445, which is the base for #568, is also not synchronised.

[dagda1]

@theKashey I think I misread you yesterday. I thought you wanted me to fix the linting errors in #568.

So I think you mean you want the isReady behaviour of the typescript plugin to be in line with the #445 and #568.

I'm going to get up to speed with the plugin. I might have a few questions but I'll try and keep it to a minimum, where is the best place to ask questions?

[theKashey]

Probably here :)

[dagda1]

@theKashey I've had a go at adding any missing tests in the loadable-ts-transformer that are in the babel-loader from loadable-components.

This highlighted this code that was not implemented:

function generateChunkNameNode(callPath: ts.CallExpression): ts.Expression {
  const importArg = getImportArg(callPath);
  if (ts.isTemplateExpression(importArg)) {
    throw new Error('not implementd');
    // return t.templateLiteral(
    //   importArg.node.quasis.map((quasi, index) =>
    //     transformQuasi(
    //       quasi,
    //       index === 0,
    //       importArg.node.quasis.length === 1,
    //     ),
    //   ),
    //   importArg.node.expressions,
    // )
  } else if (ts.isStringLiteral(importArg) || ts.isNoSubstitutionTemplateLiteral(importArg)) {
    return ts.createStringLiteral(moduleToChunk(importArg.text));
  }
  return importArg;
}

The commented out code is a copy and paste from the babel plugin. There are very little docs about the compiler API but I've got this at least creating the snapshot:

function replaceTemplateSpan(str: string, stripLeftHyphen: boolean) {
  if (!str) {
    return '';
  }

  const result = str.replace(WEBPACK_PATH_NAME_NORMALIZE_REPLACE_REGEX, '-');
  if (!stripLeftHyphen) {
    return result;
  }

  return result.replace(MATCH_LEFT_HYPHENS_REPLACE_REGEX, '');
}

function transformTemplateSpan(span: TemplateSpan, first: boolean, single: boolean) {
  const chunkName = ts.createStringLiteral(
    single ? moduleToChunk(span.getFullText()) : replaceTemplateSpan(span.getFullText(), first),
  );
  return ts.createTemplateSpan(chunkName, span.literal);
}

function generateChunkNameNode(callPath: ts.CallExpression): ts.Expression {
  const importArg = getImportArg(callPath);

  if (ts.isTemplateExpression(importArg)) {
    const templateSpans = importArg.templateSpans.map((span, index) =>
      transformTemplateSpan(span, index === 0, importArg.templateSpans.length === 1),
    );

    return ts.createTemplateExpression(importArg.head, templateSpans);
  } else if (ts.isStringLiteral(importArg) || ts.isNoSubstitutionTemplateLiteral(importArg)) {
    return ts.createStringLiteral(moduleToChunk(importArg.text));
  }
  return

This creates this snapshot:

exports[`transformer aggressive import should work with destructuration 1`] = `
"function __loadable_isReady__(self, props) {
  if (typeof __webpack_modules__ !== \\"undefined\\") {
    return !!__webpack_modules__[self.resolve(props)];
  }

  return false;
}
function __loadable_requireSync__(self, props) {
  var id = self.resolve(props);
  if (typeof __webpack_require__ !== 'undefined') {
    return __webpack_require__(id);
  }
  return eval('module.require')(id);
}
loadable({
    chunkName() {
        return \`./\${\\"foo\\"}\`;
    },
    isReady(props) {
        return __loadable_isReady__(this, props);
    },
    requireAsync: ({ foo }) => import(/* webpackChunkName: \\"\\" */ \`./\${foo}\`),
    requireSync(props) {
        return __loadable_requireSync__(this, props);
    },
    resolve() {
        return require.resolveWeak ? require.resolveWeak(/* webpackChunkName: \\"\\" */ \`./\${foo}\`) : eval(\\"require.resolve\\")(/* webpackChunkName: \\"\\" */ \`./\${foo}\`);
    }
});
"
`;

which is different from the babel plugin snapshot

exports[`plugin aggressive import should work with destructuration 1`] = `
"loadable({
  resolved: {},

  chunkName({
    foo
  }) {
    return \`\${foo}\`.replace(/[^a-zA-Z0-9_!§$()=\\\\-^°]+/g, \\"-\\");
  },

  isReady(props) {
    const key = this.resolve(props);

    if (this.resolved[key] !== true) {
      return false;
    }

    if (typeof __webpack_modules__ !== 'undefined') {
      return !!__webpack_modules__[key];
    }

    return false;
  },

  importAsync: ({
    foo
  }) => import(
  /* webpackChunkName: \\"[request]\\" */
  \`./\${foo}\`),

  requireAsync(props) {
    const key = this.resolve(props);
    this.resolved[key] = false;
    return this.importAsync(props).then(resolved => {
      this.resolved[key] = true;
      return resolved;
    });
  },

  requireSync(props) {
    const id = this.resolve(props);

    if (typeof __webpack_require__ !== 'undefined') {
      return __webpack_require__(id);
    }

    return eval('module.require')(id);
  },

  resolve({
    foo
  }) {
    if (require.resolveWeak) {
      return require.resolveWeak(\`./\${foo}\`);
    }

    return eval('require.resolve')(\`./\${foo}\`);
  }

});"
`;

I take it I should be trying to make the ts-transformer output the same code as the babel-plugin transformer?

[theKashey]

Ideally - yes. But that's hard to do, and actually would not help - it should be a unit test, which import transformed with ts or babel plugin is passing.

• existence of a expected methods
• some details about how isReady working.

I would create such test for babel one, and then ask you to adopt ts. Deal?

[dagda1]

that would be great if there were a common set of outputs i had to match.

On Sun, 17 May 2020 at 00:13, Anton Korzunov [e-mail]
wrote:

Ideally - yes. But that's hard to do, and actually would not help - it
should be a unit test, which import transformed with ts or babel plugin is
passing.

• existence of a expected methods
• some details about how isReady working.

I would create such test for babel one, and then ask you to adopt ts. Deal?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/gregberge/loadable-components/issues/458#issuecomment-629717995,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAA44OG2BJTQRS3ZRW5SIB3RR4MYNANCNFSM4JIEWXXQ
.

>

Cheers

Paul Cowan

Cutting-Edge Solutions (Scotland)

blog: http://thesoftwaresimpleton.com/
website: https://cutting.scot/

[dagda1]

Hi,
I've nearly got the plugin/transformer on my fork "working" in parity with the babel-plugin on loadable-components branch async-558 although I have not tested the actual generated code works.  As you previously mentioned, snapshots are not giving much feedback about that.

Do you have any recommendations of how I can test this out?
Failing that, maybe you could give this snapshot a quick scan for anything obvious.

The one remaining case I have is for this

  describe('loadable.lib', () => {
    it('should be transpiled too', () => {
      const result = testPlugin(`
        loadable.lib(() => import('moment'))
      `);

      expect(result).toMatchSnapshot();
    });
  });

I am completely in the dark as to what loadable.lib is.

[theKashey]

Snapshot is looking good 👍
loadable.lib is a _non-component_ wrapper around import, letting you code split generic libraries, like moment and consume they in form of render props.
Plugin works for them in the absolutely same way. Technically speaking loadable.lib is a render prop for loadable, nothing more.

[dagda1]

@theKashey I have pretty much brought my fork up to date with the tests and the code on the async-558 branch.

I've got one test that is removing all comments and not just the loadable one but it should be generating the same code although it is hard to tell with snapshot testing.

I have got a couple of TODOs in the code also.

The typescript compiler API is definitely seriously more verbose than the babel counterpart but I understand what is going on now.

[dagda1]

here is the snapshot that the code is generating. I have removed the global functions that the original repo was generating and done everything inline.

[chris-lock]

Really excited to see someone working on this. I think I might have already found a bug in your approach @dagda1. You’re calling createStringLiteral with an already quoted string, which results in an extra set of quotes instead of the single set of quotes from the babel-plugin.

A rather blunt solution would be:

return ts.createStringLiteral(
  importArg.getFullText().replace(/['"]+/g, '')
);

Not sure if the typescript provides a better utility.

[dagda1]

@chris-lock great spot, I'll fix that. The typescript compiler api is pretty nasty so appreciate any help on this. Better assertions for the tests will be great when that lands.

[theKashey]

Yeah, the ongoing issues are a clear indicator that our tests did miss something.

[chris-lock]

Hate to keep dropping bugs on this thread. Found another bug, where an empty object will throw a runtime error. Happy to open PRs for these.

Also, @theKashey, noticed that lazy isn’t supported by the babel-plugin or the Typescript transformer. Should we add support for that here since React is working to support suspense and lazy server side?

I ask because it seems like a pretty trivial change:

return (
  identifier.text === 'loadable'
  || identifier.text === 'lazy'
);

[dagda1]

@chris-lock do not apologise, thank you for pointing this out.

As far as the first bug goes, I think this is the fix:

function getCallValue(importArg: ts.Expression) {
  if ([SyntaxKind.TemplateExpression, SyntaxKind.BinaryExpression, SyntaxKind.StringLiteral].includes(importArg.kind)) {
    return importArg;
  }

  return ts.createStringLiteral(importArg.getFullText());
}

So we only create a string literal if we have to.

I'm still finding my way with this stuff and I'm also not up to speed at all about what the generated code is doing. I can see it is creating an object to be called by the loadable-components core from the LOADABLE comments

I need to dig in more, it is very interesting.

[dagda1]

@chris-lock more than happy for a PRs, how does this one get triggered:

if (getLeadingComments(node.properties[0])?.some(comment => comment.includes(LOADABLE_COMMENT))) {
  removeMatchingLeadingComments(node.properties[0], ctx, /\#__LOADABLE__/g);

  return true;
}

Do you mean If node.properties is an empty array. That code definitely looks suspect. I definitely meant to revisit that.

Another one that I have yet to fix is this test

should remove only needed comments

I'm currently just blitzing all comments after they have been read.

[dagda1]

@chris-lock do you have an example of how that bug gets triggered?

[dagda1]

@theKashey i never feel good asking oss maintainers something like this but remember you mentioned about adding a test or two here.

[theKashey]

Yep. Still remember. The new version of loadable was released yesterday and now the new iteration could be started. With new tests included.

[dagda1]

@theKashey nice one! On it

[chris-lock]

Sorry for dropping off. I believe the issue is that node.properties[0] is more accurately type as node?: ts.Node or node: ts.Node | undefined when being passed to getLeadingComments. If the ObjectLiteralExpression is empty, I believe node.properties will be an empty array, which will cause visitEachLeadingComments to be called with undefined.

[dagda1]

@chris-lock don't worry about it, I dropped off too. I understand the bug, I mean if you had code example that triggers the bug.

There are some tests now in the main repo that I am going to add

[chris-lock]

I wasn’t able to determine the offending code when I encountered it.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[dagda1]

I actually had no idea this existed.

Seems up to date also.

[theKashey]

@dagda1 - how you are comfortable with typescript plugins in overall? It seems that "platform" is not quite going to support them, and the first babel-macro based plugin will force you to use babel to handle everything (and probably TypeScript as well 😭)

[dagda1]

@theKashey I really wish babel had not got involved with typescript at all. Babel does transpile only and does not type check.

I have been using this for months now which is a fork of the original with named exports and brought up to date with loadable (before I dropped the ball :)). I totally dropped the ball due to me taking on too much work.

Is it worth pursuing do you think?

[theKashey]

Babel does transpile only and does not type check.

Typecheck and compile can be different steps. I truly believe that only _type-aware_ TS can properly compile TS, but sometimes you have to be "babel-first".
Personally I was using "first TS, then babel", but babel macros forced me to keep only babel on a few projects. The problem is literally "this issue" - ecosystem does not provide transformations for TS.

It would not be that bad is TS will support transformers in some way, but there are no signs that it is going to. For example, initially, _typescript-first_ compiled-css dropped TS support a while ago 😭

Is it worth pursuing do you think?

I could not answer this question. @madou - may be you can?

[dagda1]

@theKashey I'm firmly in the typescript first camp. I do get the impression that typescript does not want you to use their compiler API and it is pretty nasty.

[madou]

Babel does transpile only and does not type check.

Typecheck and compile can be different steps. I truly believe that only _type-aware_ TS can properly compile TS, but sometimes you have to be "babel-first".
Personally I was using "first TS, then babel", but babel macros forced me to keep only babel on a few projects. The problem is literally "this issue" - ecosystem does not provide transformations for TS.

It would not be that bad is TS will support transformers in some way, but there are no signs that it is going to. For example, initially, _typescript-first_ compiled-css dropped TS support a while ago 😭

Is it worth pursuing do you think?

I could not answer this question. @madou - may be you can?

I wouldn't suggest following it. The architecture isn't made to scale, it's not officially supported to consumer transformers, there's a lot of weird bugs that dont have a good story for fixing (the general sentiment is "it's not a bug").

I wrote a tweet about why compiled dropped it here: https://twitter.com/itsmadou/status/1290203560576036864?s=19

[dagda1]

@madou it is kind of hard to argue against anything you said in the tweet.

They have barely exposed the API and they don't want anyone to really use it.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.