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

RFC: TypeScript plugin #718

Closed
yyx990803 opened this Issue Jan 11, 2018 · 34 comments

Comments

Projects
None yet
@yyx990803
Member

yyx990803 commented Jan 11, 2018

The TypeScript plugin now has a preliminary implementation here with:

  • ts-loader
  • fork-ts-checker-webpack-plugin
  • tslint

TODOs

  • Make the npm run lint script work with *.vue files
  • Ensure compatibility with other plugins (PWA and test plugins)
  • Offer options to use TS alongside Babel

Trying it Out

npm install -g @vue/cli
vue create ts-project
# select manual mode, pick TypeScript

For background on how plugins work in the new vue-cli, see here

Feedback welcome!

/cc @DanielRosenwasser @ktsn @HerringtonDarkholme @octref @kaorun343 @Igogrek @prograhammer @Toilal

@yyx990803 yyx990803 added discussion 3.0 RFC and removed discussion labels Jan 11, 2018

@Igogrek

This comment has been minimized.

Igogrek commented Jan 11, 2018

Looks great, but wasn't able to run due to the same problem as in this thread #717.
Had to use node ..\@vue\cli\bin\vue create test-app, but yarn serve still gives me:
'vue-cli-service' is not recognized as an internal or external command

Minor note for the code:
App.vue has import like this import HelloWorld from './components/HelloWorld.vue';
But Home.vue has import HelloWorld from '@/components/HelloWorld.vue';
Which is inconsistent.
Also note that I couldn't tweak any of my IDE (WebStorm/VSCode) to properly understand @ alias.
There are some ways to ignore this as error, but it doesn't seem to work with refactorings (which is why I don't personally use it at all and still prefer longer ../../paths even for large projects).

@zeratulmdq

This comment has been minimized.

zeratulmdq commented Jan 13, 2018

Awesome work, I'll take a look tomorrow. Been waiting for this!

@zeratulmdq

This comment has been minimized.

zeratulmdq commented Jan 14, 2018

I gotta say this is the first time I use Vue with TS (been using TS with Angular for a while). I read this guide and the official TS guide so most of what I'm about to say might be just my lack of experience.

I set up the project and everything is working. I still need to fully understand the way this plugin system is gonna work. It wasn't complicated to set up everything but I would have expected a tslint/tsconfig file out of the box and the addition of "@vue/cli-plugin-typescript": "3.0.0-alpha.1", to package.json. I guess I must be missing something (some command maybe to tell the project I wanna use TS).

Regarding TS itself, I understand Vue converts properties inside the data object into getters/setters so this might be kind of complicated to acomplish without modifying internal things but I would expect something like this to throw:

import {Person} from '../models/person';

interface Data {
  msg: string;
  people: Person[];
}

export default {
  name: 'HelloWorld',
  data(): Data {
    return {
      msg: 'Welcome!',
      people: []
    }
  },
  created() {
    this.people = someService.randomNumber();
  }
}
</script>

I mean, I'm typing data() but that's just for the initial rendering. Inside the created hook I'm assigning a number to a previously typed data property and it will work. Is there a workaround for that?

BTW, as I always say, awesome work.

@Igogrek

This comment has been minimized.

Igogrek commented Jan 15, 2018

@zeratulmdq
Good point, but until typings are improved for such cases the only good way to proceed is to use vue-class-component (there's an option already).
This way you can declare msg and people as class properties and error will be thrown.

@Toilal

This comment has been minimized.

Toilal commented Jan 15, 2018

Use export default Vue.extend({}) for better TypeScript integration into Vanilla API (without vue-class-component). (Since Vue 2.5)

@Toilal

This comment has been minimized.

Toilal commented Jan 15, 2018

I just tested generating a TypeScript project and a default project.

This is a really good base and seems really simpler to bring a custom feature into vue-cli.

Main difference from stable version is that no webpack configuration file is generated. I see two drawbacks for this choice :

  • It may break some editors features.

For example, IntellijIDEA can parse webpack configuration like webpack aliases. So in IntelliJ IDEA with default template (no typescript), when I write import HelloWorld from '@/components/HelloWorld.vue', it shows a warning and the module is not resolved in the editor (=> no completion, no navigation, etc ...). With TypeScript option, it works because it relies on tsconfig.json compilerOptions.paths.

  • It may be hard for a end-user that is not involved in vue-cli project to add an additional webpack plugin/loader.
@zeratulmdq

This comment has been minimized.

zeratulmdq commented Jan 15, 2018

Thanks @Toilal @Igogrek I'll take a look

@yyx990803

This comment has been minimized.

Member

yyx990803 commented Jan 15, 2018

@Toilal you can access the webpack config as a file via node_modules/vue-cli-service/webpack.config.js

We will have a a ton of examples and recipes teaching users how to tweak the webpack config for common use cases.

@octref

This comment has been minimized.

Member

octref commented Jan 18, 2018

@yyx990803 Will writing plugins that use webpack-chain the main way to tweak webpack config?
Since changes made to node_modules/vue-cli-service/webpack.config.js will be lost after reinstall deps...right?

Other than that, PWA + TS seems a pretty solid setup for me. Will use it for a toy project to see how it goes and give more feedback.

@yyx990803

This comment has been minimized.

Member

yyx990803 commented Jan 27, 2018

Update: we have started making alpha releases! You can now directly test the new CLI by following the guide in README.

@AlexanderVeber

This comment has been minimized.

AlexanderVeber commented Jan 29, 2018

Can you add tslint-config-standard as an option when enabling TSLint?

@franxois

This comment has been minimized.

franxois commented Feb 1, 2018

Hi,
great work ! I don't know if it the right place but here is my tests

npx @vue/cli create vue-ts-test

Vue CLI v3.0.0-alpha.5
? Please pick a preset: Manually select features
? Check the features needed for your project: TS, PWA, Router, Vuex, CSS Pre-processors, Linter, Unit, E2E
? Use class-style component syntax? Yes
? Use Babel alongside TypeScript for auto-detected polyfills? No
? Pick a CSS pre-processor (PostCSS, Autoprefixer and CSS Modules are supported by default): LESS
? Pick additional lint features: 
? Pick a unit testing solution: Jest
? Pick a E2E testing solution: Cypress
? Where do you prefer placing config for Babel, PostCSS, ESLint, etc.? In dedicated config files
? Save this as a preset for future projects? No

If I run npm run serve it show errors like below but i can browse and hot reload works

ERROR in /tmp/vue-ts-test/node_modules/@types/jest/index.d.ts
18:13 Duplicate identifier 'beforeEach'.
    16 | 
    17 | declare var beforeAll: jest.Lifecycle;
  > 18 | declare var beforeEach: jest.Lifecycle;
       |             ^
    19 | declare var afterAll: jest.Lifecycle;
    20 | declare var afterEach: jest.Lifecycle;
    21 | declare var describe: jest.Describe;
 

̀npm run build fails

@yyx990803

This comment has been minimized.

Member

yyx990803 commented Feb 1, 2018

@franxois looks like a type conflict between Cypress and Jest. We will need explicit tsconfig.json inside test directories.

Tracked in #762

@franxois

This comment has been minimized.

franxois commented Feb 2, 2018

Hi, i just try with v3.0.0-alpha.6, it's ok now ! :)

@yyx990803

This comment has been minimized.

Member

yyx990803 commented Feb 2, 2018

@DanielRosenwasser I think we are more or less stable now, would love to get your feedback on this before we move to beta :)

@Toilal

This comment has been minimized.

Toilal commented Feb 3, 2018

I just gave a try to the new generator (alpha 7) with all options checked (including TypeScript). It runs smoothly and is zero-conf for opening the generated project in IntelliJ.

Class component option could be available without TypeScript too.

When disabling TypeScript, I have to configure the webpack configuration file in IntelliJ (node_modules/@vue/cli-service/webpack.config.js) for @ import alias to be recognized, but then all is OK. I think custom configuration placed in vue.config.js won't be available to IntelliJ.

Maybe the mutated webpack configuration could be written to a file so the effective final configuration can be loaded by tools/editors ?

Thanks for the awesome work 👍

@yyx990803

This comment has been minimized.

Member

yyx990803 commented Feb 3, 2018

@Toilal that webpack file exports the dynamically resolved config that includes your custom config as well ;)

@dkulchenko

This comment has been minimized.

dkulchenko commented Feb 7, 2018

Looks awesome!

Would it be possible to have a choice between TSLint and ESLint, even when in TypeScript mode?

I much much prefer working with ESLint even on my TypeScript Vue projects (better VSCode support, eslint-plugin-vue doesn't have a TS equivalent, etc.). Tried it both ways and I've found life to just be a lot easier with ESLint at the moment.

It's a relatively straightforward integration - it means instead of adding tslint and tslint.json, keeping eslint as normal and adding the typescript-eslint-parser and eslint-plugin-typescript npm deps, then taking eslintOptions.js from cli-plugin-eslint and changing a few options:

  extensions: ['.js', '.ts', '.vue'],
  parserOptions: {
    parser: require.resolve('typescript-eslint-parser'),
    sourceType: 'module',
  },
  plugins: ['typescript'],

Also adding the ts extension to the webpack eslint-loader test in cli-plugin-eslint, and tweaking a few enabled eslint rules, but all else should be essentially untouched.

@yyx990803

This comment has been minimized.

Member

yyx990803 commented Feb 7, 2018

@dkulchenko cool, I actually didn't know you can use ESLint with TypeScript before. I think that's indeed a better setup than TSLint. Will investigate in the next release.

@ajafff

This comment has been minimized.

ajafff commented Feb 9, 2018

@yyx990803 @dkulchenko would you consider using a completely different linter instead?

TSLint currently doesn't support .vue files. It's unlikely that this feature will be added anytime soon as it requires a major rewrite. In addition the project seems to be abandoned by its owners. That's why I'm no longer working on it.

IIRC ESLint with typescript-eslint-parser has problems to provide correct type information because every file is checked in isolation.

I'm currently working on a replacement for TSLint that can be fully customized: https://github.com/fimbullinter/wotan
There's also a processor for vue files (similar to eslint-plugin-vue): https://github.com/fimbullinter/wotan/blob/master/packages/ve/README.md
And you can optionally use existing TSLint core rules as well as custom rules: https://github.com/fimbullinter/wotan/blob/master/packages/heimdall/README.md

@DanielRosenwasser

This comment has been minimized.

Contributor

DanielRosenwasser commented Feb 10, 2018

@yyx990803 I will try to stress test it soon!

I think one thing that would be super high-value would be a lint rule to warn Vue+TypeScript users about computed properties, where all methods and get/set accessors need to have explicit type annotations to avoid inference problems with TypeScript (vuejs/vue#7394). Whatever linter we end up choosing, we should factor that into the starter.

@yyx990803

This comment has been minimized.

Member

yyx990803 commented Feb 10, 2018

@DanielRosenwasser it's possible to use either ESLint (via typescript-eslint-parser) or TSLint, but I assume the implementation of the rule would be similar. I think in the long run we'd like to standardize around ESLint because that allows linting of templates inside SFCs, and the rule you proposed could be placed in https://github.com/vuejs/vue-cli/tree/dev/packages/%40vue/eslint-config-typescript

/cc @mysticatea @michalsnik

@mysticatea

This comment has been minimized.

Member

mysticatea commented Feb 10, 2018

Cool.

In fact, the combination of vue-eslint-parser and typescript-eslint-parser needs more work a bit. It needs that typescript-eslint-parser supports parseResult.visitorKeys.

Currently, I'm busy on acorn and eslint to support ES2018, but I will work on it after done.

@prograhammer

This comment has been minimized.

prograhammer commented Feb 12, 2018

@mysticatea @dkulchenko @yyx990803 Does the ESLint plugin support rules that use the type checker?

@yyx990803

This comment has been minimized.

Member

yyx990803 commented Feb 12, 2018

@prograhammer AFAIK, no. With the current setup, type checking is performed by fork-ts-checker-webpack-plugin in a separate thread, and we don't even enable type checker depending rules when using TSLint.

@prograhammer

This comment has been minimized.

prograhammer commented Feb 12, 2018

Ah, ok. Now I'm wondering why you wouldn't enable tslint in fork-ts-checker-webpack-plugin, but it looks like you need the ability to tslint fix files or run tslint separately from fork-ts-checker-webpack-plugin?

@prograhammer

This comment has been minimized.

prograhammer commented Feb 12, 2018

@yyx990803 fork-ts-checker-webpack-plugin can pass the program over (the program will read .vue files in a way that line/column positions are preserved) to tslint. You just enable the tslint flag in fork-ts-checker-webpack-plugin. What repo is generating the webpack config in this new Vue Cli setup?

@yyx990803

This comment has been minimized.

Member

yyx990803 commented Feb 12, 2018

@prograhammer actually, when using lintOnSave: true, tslint is done via fork-ts-checker-webpack-plugin, and I think that does work with type-checker-dependent rules.

@DanielRosenwasser

This comment has been minimized.

Contributor

DanielRosenwasser commented Feb 16, 2018

Hey @yyx990803, since you already have support for TSLint, is there a specific place we should place the rule? I know that the respective ESLint rule should probably go in https://github.com/vuejs/eslint-plugin-vue

@yyx990803

This comment has been minimized.

Member

yyx990803 commented Feb 16, 2018

@DanielRosenwasser currently TSLint is handled inside the TypeScript plugin itself and we are just using tslint:recommended. We don't really have a TSLint config at the moment. Maybe put it as a custom rule in a rulesDirectory for now?

@prograhammer

This comment has been minimized.

prograhammer commented Feb 17, 2018

@yyx990803 @DanielRosenwasser
I personally use tslint-config-standard (set in a tslint.json), which requires the type-checker for some rules.

@yyx990803 yyx990803 removed the 3.0 label Mar 1, 2018

@do-web

This comment has been minimized.

do-web commented Mar 1, 2018

Could you please add typescript support to "vue serve MyComponent.vue" maybe with a parameter "vue serve -ts MyComponent.vue"

@DrSensor

This comment has been minimized.

DrSensor commented Mar 4, 2018

I also love vue serve -ts to be implemented because when I work with medium-large project that use typescript everywhere, it consume a lot of RAM 😂

@ErikAllan

This comment has been minimized.

ErikAllan commented Mar 11, 2018

I've just tried the typescript template and everything works perfect so far, except for one thing... How do I use typescript in the service worker file? I use this setting in the vue.config.js file to include the worker:

module.exports = {
pwa: {
workboxPluginMode: 'InjectManifest',
workboxOptions: {
swSrc: 'src/sw.js',
},
},
};

I would like to replace the sw.js file with a sw.ts file, but Workbox doesn't seem to allow that. I have a lot of code inside the service worker that needs to be written in typescript...

If this is the wrong forum for this question, please redirect me :-)

@yyx990803 yyx990803 closed this Apr 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment