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

Example for preset-typescript with vue-cli and potential fix #122

Closed
wants to merge 4 commits into from

Conversation

graup
Copy link
Contributor

@graup graup commented Mar 28, 2020

I added another example, which is very close to what vue-cli produces when you initialize a new project with typescript and babel. It doesn't work with the current version (3.0.0) – see #121

While digging around, it seems the babel-loader doesn't apply the custom babel config correctly (which in this case is needed for the class properties and decorators), or it does so at the wrong time, or something.

From the previous version, I salvaged the code that added ts-loader, and sure enough, that works. I added this code back in for testing. Ideally, we don't need it, but if we can't find another solution, this may be the easiest fix.

/edit: when using @vue/cli-plugin-babel/preset, you don't need babel-preset-typescript-vue anymore, but we should probably leave it in for folks who don't have their own babel config. It doesn't seem to conflict.

@shilman
Copy link
Member

shilman commented Mar 29, 2020

Thanks for looking into this @graup!

Any idea why babel is failing? Does this code first run babel on the user's code and THEN run typescript on the output?

I know this is already a fallback, but if we want people to switch to babel, I'd suggest only add this if framework is "vue", or maybe if the user explicitly defines tsLoaderOptions (opt-in).

WDYT? cc @Aaron-Pool @mrmckeb

@shilman shilman added bug Something isn't working typescript labels Mar 29, 2020
@graup
Copy link
Contributor Author

graup commented Mar 29, 2020

I'm not sure. As far as I understand, the webpack chain is:

(.vue) vue-loader -> (.ts) ts-loader -> (.js) babel-loader

This seems to go back to the issue that @mrmckeb was fighting – maybe there just isn't any sensible way to use Vue's SCF without ts-loader :/
N.B. that vue-cli is using ts-loader (optionally additionally with babel-loader), so maybe it's not a good idea to go against that: https://github.com/vuejs/vue-cli/blob/137e7a724eae7a4e2e84dd3e8d4c774567c3c142/packages/%40vue/cli-plugin-typescript/index.js#L51-L77

/edit: on the other hand, vue class components docs say that it should be possible to use it with only babel...

@shilman
Copy link
Member

shilman commented Mar 29, 2020

OK, I'm cool with that:

  1. if that's the best we can come up with
  2. if we make sure that ts-loader is either opt-in, or only applied in the "vue" case

Having both loaders running in React will be confusing and possibly problematic.

@mrmckeb
Copy link
Member

mrmckeb commented Mar 31, 2020

Sorry guys, I checked out for a few days.

I'll chat to @shilman on this and get back to you ASAP. Sorry, we did try to verify that this was working with Vue, but apparently didn't test thoroughly enough.

@shilman
Copy link
Member

shilman commented Apr 7, 2020

@graup what do you think about building this into the vue framework preset in @storybook/vue? i talked to @mrmckeb and he said the vue typescript story is pretty unique and would justify that, rather than putting it into the preset.

@rcidaleassumpo
Copy link

rcidaleassumpo commented Apr 8, 2020

edit: I just realized that this isn't merged. So what's the status on this ? Thanks.
Hi, since this is merged, does that mean that in the near future the preset-typescript will be working with vue, or ? What's the step to make typescript and vue work together ? Thank you.

@dgreene1
Copy link

Btws, the readme still describes that you can pass a ts-loader config object even though it appears that ts-loader was removed in a prior PR but is being proposed to be brought back in this PR.

@mrmckeb
Copy link
Member

mrmckeb commented Apr 16, 2020

Sorry I should update here. I've had a discussion with @graup and he agrees that we should move this to the framework.

@shilman that means we could close this off in favour of adding this functionality to the Vue framework preset. @graup could you do that work? Or I can take a look?

@shilman
Copy link
Member

shilman commented Apr 16, 2020

SGTM. Closing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working typescript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants