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

feat: Support vue.config.ts #6820

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from

Conversation

screetBloom
Copy link
Contributor

@screetBloom screetBloom commented Nov 8, 2021

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Underlying tools
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

Other information

It would be great to use .ts suffix configuration files in TypeScript projects

image

@screetBloom
Copy link
Contributor Author

/vue.config.js has been mocked, and the test error here is confusing

https://github.com/vuejs/vue-cli/blob/dev/packages/%40vue/cli-service/__tests__/Service.spec.js#L134-L135

image

Copy link
Member

@sodatea sodatea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding the usage of esbuild:
Seems overkill for an opt-in feature. How about using typescript.transpileModule and list typescript as an optional peer dependency (which will be installed if the user installs @vue/cli-plugin-typescript)?

Regarding the test error:
Maybe caused by deleting require.cache?
Anyway, it seems a jest-only quirk. You can add whatever workaround you need in the if (process.env.VUE_CLI_TEST condition to make jest happy.

@screetBloom
Copy link
Contributor Author

screetBloom commented Nov 10, 2021

Regarding the usage of esbuild: Seems overkill for an opt-in feature. How about using typescript.transpileModule and list typescript as an optional peer dependency (which will be installed if the user installs @vue/cli-plugin-typescript)?

Regarding the test error: Maybe caused by deleting require.cache? Anyway, it seems a jest-only quirk. You can add whatever workaround you need in the if (process.env.VUE_CLI_TEST condition to make jest happy.

Okay, I'll think about your suggestion and Solve the error of jest

@screetBloom
Copy link
Contributor Author

screetBloom commented Nov 10, 2021

One of the reasons I use esbuild is that I want to replace a lot of built-in feature with it

for example

  • refactoring the Vue CLI with typescript
  • use esbuild-jest to replace jest
  • replace the minify of img/css/js with esbuild by default

@sodatea
Copy link
Member

sodatea commented Nov 10, 2021

Yeah, I agree there are a lot of possibilities if we bring in esbuild.
But I feel that it can be opt-in in this scenario.

We can add it later when we have more convincing use cases.

@sodatea
Copy link
Member

sodatea commented Nov 10, 2021

  • refactoring the Vue CLI with typescript

In this case, esbuild is a dev dependency.

  • use esbuild-jest to replace jest

This change won't improve the test speed of the Vue CLI project itself.

For user projects, yeah, I think this use case is worth investigating. But we can implement a separate jest preset with esbuild-jest first and integrate it into CLI core later after real-world testing.

  • replace the minify of img/css/js with esbuild by default

Not now. It must be opt-in for Vue CLI at this moment because the speed of esbuild comes with tradeoffs.
Users who explicitly enable esbuilld minification would know the bugs may come from esbuild.
But most others won't know it.
I don't feel like answering questions on compatibility issues in Vue CLI.

@screetBloom
Copy link
Contributor Author

screetBloom commented Nov 10, 2021

The reason why we do not want to add esbuild may be worried about the size of node_modules, but in fact its size is not large

yarn init -y && yarn add esbuild

open node_modules

This size is all its influence on node_moduels 👇, But the size of typescript is 63M


The size change of 8M is not difficult to accept for user


For Vue CLI5, I think we can add some current popular tools. This is a growing development trend 😄, and in the future we don’t need to replace the tools developed based on typescript with esbuild.


I think we can introduce changes more optimistically


@sodatea
Copy link
Member

sodatea commented Nov 10, 2021

There's a bug in yarn v1 that it will download all dependencies regardless of their platform field.
So a fresh install will download about 50MB of data from the registry.

@sodatea
Copy link
Member

sodatea commented Nov 10, 2021

But the size of typescript is 63M

But typescript is a peer dependency. esbuild would be built-in. For a feature that only typescript users need.

@screetBloom
Copy link
Contributor Author

There's a bug in yarn v1 that it will download all dependencies regardless of their platform field. So a fresh install will download about 50MB of data from the registry.

image

@sodatea
Copy link
Member

sodatea commented Nov 10, 2021

For Vue CLI5, I think we can add some current popular tools. This is a growing development trend 😄, and in the future we don’t need to replace the tools developed based on typescript with esbuild.

I'm more than happy to accept these changes. But I prefer an opt-in way because of the maintenance burden.

For example, it would be cool to have an esbuild option alongside the babel plugin, for those who don't have to support IE11; it might be easier to maintain if the jest plugin uses esbuild-jest instead of babel-jest and ts-jest for its builtin presets.

But I don't see any immediate benefit of including it in @vue/cli-service.

@sodatea
Copy link
Member

sodatea commented Nov 10, 2021

There's a bug in yarn v1 that it will download all dependencies regardless of their platform field. So a fresh install will download about 50MB of data from the registry.

image

check your cache: yarn cache list --pattern esbuild

@screetBloom
Copy link
Contributor Author

Let me think about how to convince you🤔.

If it is not possible I , I will use transpileModule first. And mention a separate esbuild optimization pr next time

@xiaoxiangmoe
Copy link
Contributor

@screetBloom Can you also add vue.config.mts and vue.config.cts

@screetBloom
Copy link
Contributor Author

screetBloom commented Nov 23, 2021

@sodatea

Can this point add some convincing ?
In non-ts projects it is also possible for user to use ESM in vue.config.js files And use it in much the same way as vite

this looks a little more modern, .mjs is easily confusing for many user, they need to understand that the runtime of vue.config.mjs is node, so it needs to be the .mjs suffix


see here 👉 vite config file resolving

image

@sodatea
Copy link
Member

sodatea commented Nov 24, 2021

I finally found out why I don't feel right about this implementation:

Vite's implementation is not the ideal example.
Using esbuild.build to bundle the config file doesn't work for external .ts files (e.g., in a monorepo).

We really don't need esbuild for this simple feature.
A simple ts-node/register-like utility would suffice.


Again, I'm not against adding esbuild to Vue CLI. I'm just against using it for this feature.

@screetBloom
Copy link
Contributor Author

screetBloom commented Nov 24, 2021

I finally found out why I don't feel right about this implementation:

Vite's implementation is not the ideal example. Using esbuild.build to bundle the config file doesn't work for external .ts files (e.g., in a monorepo).

We really don't need esbuild for this simple feature. A simple ts-node/register-like utility would suffice.

Again, I'm not against adding esbuild to Vue CLI. I'm just against using it for this feature.


Okay, one last question, do you prefer to use typescript.transpileModule or ts-node/register 😄

In fact, at first I use ts-node/register in my company, but ts-node is not in the dependencies or peerDependencies of @vue/cli-service

I will finish it as soon as possible

@sodatea
Copy link
Member

sodatea commented Nov 25, 2021

Either is fine to me. Whichever you choose, please add it to the peer dependencies list and mark it as optional.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants