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

Possibility to support vue-loader? #270

Closed
HerringtonDarkholme opened this issue Aug 23, 2016 · 14 comments

Comments

Projects
None yet
2 participants
@HerringtonDarkholme
Copy link
Contributor

commented Aug 23, 2016

Thanks for building ts-loader for webpack! I wonder whether ts-loader can support vue.js' single file component.

Related issue in vue-loader
vuejs/vue-loader#109

The current problem is ts-loader cannot resolve files with extension name .vue. However, TypeScript compiler supports customizing module resolution.

Since ts-loader has already implemented a custom resolveModuleNames function, supporting it is feasible.

I tried locally and it works. However I'm a newbie to webpack and I think my try is dirty and hacky...

Do you have any good ideas? I'm happy to implement them.

@HerringtonDarkholme

This comment has been minimized.

Copy link
Contributor Author

commented Oct 24, 2016

I forked ts-loader for this. But it seems ts-loader changes quite fast. Any possibility to support vue natively in ts-loader?

@johnnyreilly

This comment has been minimized.

Copy link
Member

commented Oct 24, 2016

Quite possibly. I'm just doing a bit of a refactor on the code to get it to a more maintainable place. I'd be interested in getting vue support in once that's done. (I'm currently guessing I'll be done with the refactor inside a week or 2).

Does adding vue to your webpack.config not resolve the issue?:

 resolve: {
    extensions: ['', '.ts', '.tsx', '.js', 'vue']

Forgive me if that's a naïve question; I have 0 vue experience....

@HerringtonDarkholme

This comment has been minimized.

Copy link
Contributor Author

commented Oct 24, 2016

It still need some special handling in loader, but rather small modification.

HerringtonDarkholme/vue-ts-loader@d581d46

The core modification is appending .ts suffix to *.vue file, then every thing is fine. My concern is its hard coding nature.

@johnnyreilly

This comment has been minimized.

Copy link
Member

commented Oct 24, 2016

I'm on my phone and so can't really look at this properly. But I think we can avoid hard coding and just add another option. As I see it

function appendSuffixToVue(fileName: string) {
    if (/\.vue$/.test(fileName)) {
        return fileName + '.ts';
    }
    return fileName;
}

Is something that tests a file path ends with vue and if so suffixes .ts on the end. So what say we introduce a ts-loader option called appendTsSuffixTo that, when supplied, is used to suffix .ts in the manner your fork does. Sound good?

@HerringtonDarkholme

This comment has been minimized.

Copy link
Contributor Author

commented Oct 24, 2016

That's great. More generic. 👍

@johnnyreilly

This comment has been minimized.

Copy link
Member

commented Oct 24, 2016

Great - we'll need an integration test to cover this. You may already have one we can use. We've recently introduced "execution tests" as well as the pre-existing "comparison tests". Either will do. The comparison tests only run against the latest released version of TypeScript. The execution tests against all versions back to 1.6.

@johnnyreilly johnnyreilly added this to the 1.1 milestone Oct 29, 2016

@HerringtonDarkholme

This comment has been minimized.

Copy link
Contributor Author

commented Oct 29, 2016

I will submit a pull request once refactor is done. Looking forward to your refactor :)

@johnnyreilly

This comment has been minimized.

Copy link
Member

commented Oct 29, 2016

Cool! The refactor is basically done. Just waiting on some feedback. Will let you know when I merge 😄

@johnnyreilly

This comment has been minimized.

Copy link
Member

commented Oct 29, 2016

@johnnyreilly

This comment has been minimized.

Copy link
Member

commented Nov 1, 2016

Over to you... 1.0.0 has 🚢 😄

@HerringtonDarkholme

This comment has been minimized.

Copy link
Contributor Author

commented Nov 2, 2016

Thanks! I need some time to understand what ts-loader does now, so please be patient.

@johnnyreilly

This comment has been minimized.

Copy link
Member

commented Nov 2, 2016

No worries - I need that too!

@HerringtonDarkholme

This comment has been minimized.

Copy link
Contributor Author

commented Nov 2, 2016

Is there any flaky test? vue-loader's output is indeed very flaky...

@johnnyreilly

This comment has been minimized.

Copy link
Member

commented Nov 2, 2016

There are flaky tests in the comparison test pack. Look out for a _FLAKY_ file in the root of test directories. This denotes a flaky test and a failure of this will not fail the build. I think there's about 3 flaky tests; mainly down to platform issues. See this as an example of a flaky test:

https://github.com/TypeStrong/ts-loader/tree/master/test/comparison-tests/html-webpack-plugin

johnnyreilly added a commit that referenced this issue Nov 4, 2016

Merge pull request #354 from HerringtonDarkholme/dev
[RFC] fix #270, support `appendTsSuffixTo` config
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.