Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

TypeScript support: don't try to compile templates #1222

Closed
wants to merge 4 commits into from

Conversation

benmccann
Copy link
Member

@benmccann benmccann commented May 22, 2020

Enables TypeScript support #760

TypeScript won't compile with Svelte compiler and first needs to be preprocessed. However, that happens after this check because create_app outputs JS which imports these preload exports before we try to bundle the whole app

This also has to be much faster to build the mainfest!

@thgh
Copy link
Contributor

thgh commented May 22, 2020

Maybe add an extra check for the file extension and only disable for .ts?

There is a usecase where you may want to compose the preload function:
export const preload = verifyUser(catchErrors(function () { ... }))

Also reusable preload functions:
export { preload } from './userData'

@benmccann
Copy link
Member Author

Thanks @thgh ! I've updated the regex to cover both the cases you mentioned. Take another look and let me know what you think

Maybe add an extra check for the file extension and only disable for .ts?

The code is typically in a .svelte file, so we couldn't do that unfortunately. But I think this will be a really nice speed up for building apps in JS anyway.

@benmccann
Copy link
Member Author

I've been using this in my fork for the past few weeks and the only issue I ran into was if preload was commented out. I've updated this PR to address that case and added several new unit tests to cover all the cases that have been suggested so far.

@Conduitry
Copy link
Member

Another possibility might be https://github.com/Rich-Harris/tippex

@benmccann
Copy link
Member Author

Cool. I'm happy to switch it out if you prefer. strip-comments has been working well though and is a heavily used library

@Conduitry
Copy link
Member

Well tippex seems to match the case we have here better because it also blanks out strings from its input, which strip-comments does not but which is needed to help avoid false positives.

@PatrickG
Copy link
Member

In the meantime, maybe we can only compile the module script block, until this is resolved?

Like this:

let matches;
if (/preload/.test(source) && (matches = source.match(/<script context="module">[^]*?<\/script>/))) {
  try {
    const { vars } = svelte.compile(matches[0], { generate: false });
    return vars.some((variable: any) => variable.module && variable.export_name === 'preload');
  } catch (err) {}
}

The benefit would be, to use typescript in the other script block.

@benmccann
Copy link
Member Author

I've gone ahead and switched this to use https://github.com/Rich-Harris/tippex

@benmccann
Copy link
Member Author

The main thing I was keeping my eyes on with this PR is the fact that it uses a regex to extract the <script context="module"> tags. It turns out that svelte itself uses a regex to extract script tags, so I think it's highly unlikely that this would cause any regression. If Svelte comes up with any other way of doing this in the future then we can always update Sapper to match

https://github.com/sveltejs/svelte/blob/40987b7780ef1865fb94e89b4d2f5a53a71056f9/src/compiler/preprocess/index.ts#L97

@Conduitry
Copy link
Member

This was a good idea, but closing in favor of #1344, which is a better idea.

@Conduitry Conduitry closed this Jul 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants