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(builder): optional typescript support #4557

Merged
merged 8 commits into from Dec 15, 2018

Conversation

kevinmarrec
Copy link
Contributor

@kevinmarrec kevinmarrec commented Dec 14, 2018

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

⚠ Warning ⚠ (EDIT AFTER MERGE)

➡ Please be aware of #4563 , which will likely make this merged PR outdated. ⬅

Description

What if Typescript Support what just about setting a boolean variable to true ? 😎

Prequel

This PR is highly inspired by #4406 made by @chanlito, so special thanks to him.

His current PR state alters Nuxt packages even for non-TS projects, and will also break projects using their own TypeScript module, that's why I implemented a new way to provide the TS support.

Existing TS projects will be able to either keep their own implementation of TypeScript support OR use this built-in optional one.

I. Usage 👨‍💻👩‍💻

// nuxt.config.js
export default {
  build: {
    typescript: true // 🚀
  }
}

II. How it works behind the scenes ? ⚙

When the typescript option is set to true, and only if is set true, it alters the following Nuxt internal packages :

✏ Config (@nuxt/config)
Accept .ts(x) files for plugins and middleware

✏ Builder (@nuxt/builder) & Common(@nuxt/common)
Watch & Use .ts(x) files to generate routes (layouts & pages)

✏ Webpack (@nuxt/webpack)
Resolve .ts(x) files through Babel, passing the typescript option value to the Nuxt babel preset

✏ Babel (@nuxt/babel-preset-app)
Retrieve the typescript option from the preset options to use the @babel/preset-typescript Babel TypeScript preset.

✏ CLI (@nuxt/cli) [Suggestion not implemented yet]
Warning or Error if tsconfig.json not found in the source folder, using the srcDir config option

III. What about the documentation ? 📖

I suggest a Typescript section in the documentation OR a How to use Typescript ? in the FAQ.

I will (or at least propose to) take the lead on it if my proposal is accepted and submit a PR regarding this documentation. This latter will talk about type definitions as well (#4164).

IV. What about a Nuxt TS project example ? 📁

An example highly similar to the one in #4406 will be proposed in a different PR if this proposal is accepted.

V. What about tests ? 🛠

A test fixture and an unit test are linked in the PR.

NOTE : Nuxt TS community is growing, we already have a bunch of developers who will be ready to test this support using nuxt-edge if this proposal gets accepted and merged 😇

@chanlito @hartmut-co-uk @husayt @ksnyde @NickBolles

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly. (PR: #)
  • I have added tests to cover my changes (if not applicable, please state why)
  • All new and existing tests are passing.

WIP
TSX working 👍

Better tsconfig.json

Update fixture typings

Update fixture
@kevinmarrec
Copy link
Contributor Author

kevinmarrec commented Dec 14, 2018

It seems that I broke something around the Regex's regarding layouts & pages. I will fix it soon 💪.

EDIT : Fixed my mistakes regarding Regex's but now there is a Vue warning causing an error, I need to investigate.

@NickBolles
Copy link

@kevinmarrec Not sure if this is how this works, or if it would be an issue, but I think that this line will evaluate to true for typescript options for nuxt-ts-module

maybe do === true?

I can try to test if using nuxt from this PR will break anything in my app, but probably wont get to it for a few days.

@kevinmarrec
Copy link
Contributor Author

kevinmarrec commented Dec 14, 2018

@NickBolles Thank for finding this, it will indeed cause an issue, your suggestion or finding a different name for the option are 2 valid solutions.

@NickBolles
Copy link

NickBolles commented Dec 14, 2018

Are we ever going to want to pass options to Typescript? We might want it to be an object at some point, for example to pass a location for the tsconfig or something. (which would conflict even more with other packages)

Could we simply move it to the build object? it kinda is a build-related option.

@kevinmarrec
Copy link
Contributor Author

kevinmarrec commented Dec 14, 2018

Yeah I think it could be moved to the build object.

EDIT : I checked the documentation, the build object is related to Webpack build so typescript has obviously its place here.

@kevinmarrec kevinmarrec changed the title Provide optional TypeScript support through a new "typescript" config option Provide optional TypeScript support through a new "typescript" config build option Dec 14, 2018
pi0
pi0 previously approved these changes Dec 14, 2018
Copy link
Member

@pi0 pi0 left a comment

Choose a reason for hiding this comment

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

Generally changes (till know) LGTM. Nice works 😊

packages/builder/src/builder.js Outdated Show resolved Hide resolved
packages/builder/src/builder.js Show resolved Hide resolved
@pi0
Copy link
Member

pi0 commented Dec 14, 2018

Also good decision to move it into build.typescript. I was going to tell that :))

@kevinmarrec
Copy link
Contributor Author

kevinmarrec commented Dec 14, 2018

@pi0 Thanks for your fast feedback ! I will take into account your code comments soon and I need to fix the unit test :).

@galvez galvez changed the title Provide optional TypeScript support through a new "typescript" config build option feat: optional TypeScript support via build config option Dec 15, 2018
@galvez galvez changed the title feat: optional TypeScript support via build config option feat: optional TypeScript support via build option Dec 15, 2018
@galvez galvez changed the title feat: optional TypeScript support via build option feat: TypeScript support via build option Dec 15, 2018
@codecov-io
Copy link

codecov-io commented Dec 15, 2018

Codecov Report

Merging #4557 into dev will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #4557      +/-   ##
==========================================
+ Coverage   91.07%   91.11%   +0.03%     
==========================================
  Files          61       61              
  Lines        2186     2195       +9     
  Branches      528      533       +5     
==========================================
+ Hits         1991     2000       +9     
  Misses        177      177              
  Partials       18       18
Impacted Files Coverage Δ
packages/config/src/config/build.js 85.71% <ø> (ø) ⬆️
packages/babel-preset-app/src/index.js 100% <100%> (ø) ⬆️
packages/webpack/src/config/base.js 95.5% <100%> (+0.15%) ⬆️
packages/config/src/options.js 94.44% <100%> (+0.07%) ⬆️
packages/common/src/utils.js 98.63% <100%> (ø) ⬆️
packages/builder/src/builder.js 96.51% <100%> (+0.08%) ⬆️
packages/vue-renderer/src/spa-meta.js 85.18% <0%> (-0.27%) ⬇️
packages/vue-renderer/src/renderer.js 95.6% <0%> (-0.03%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9961453...a05b8d2. Read the comment docs.

@kevinmarrec
Copy link
Contributor Author

Tests are green 🚥✔

galvez
galvez previously approved these changes Dec 15, 2018
Copy link

@galvez galvez left a comment

Choose a reason for hiding this comment

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

You're right @kevinmarrec -- LGTM!

@pi0
Copy link
Member

pi0 commented Dec 15, 2018

Merging to get this enhancenment sooner with nuxt-edge. According to PR notes these items are delegated to other PRs:

  • Warning or Error if tsconfig.json not found in the source folder, using the srcDir config option
  • A Typescript section in the documentation OR a How to use Typescript ? in the FAQ.
  • Nuxt TS project example

@pi0 pi0 merged commit 7145c1a into nuxt:dev Dec 15, 2018
@pi0 pi0 changed the title feat: TypeScript support via build option feat(builder): optional typescript support Dec 15, 2018
@husayt
Copy link
Contributor

husayt commented Dec 16, 2018

This is not only a big time saver and very elegant solution, but is also a long overdue nuxt feature. After all, this is what nuxtjs is about, it has even TJS in the name NuxTJS. 😀

I am going to try nuxt hacketnews TS using this way and see what happens.

@kevinmarrec
Copy link
Contributor Author

@husayt There are issues with Vue SFC's using the Babel approach, i'm not sure you'll be able to make your project work ^^

@hartmut-co-uk
Copy link

hartmut-co-uk commented Dec 16, 2018

@husayt see/await #4563

@kevinmarrec
Copy link
Contributor Author

I updated the description of this PR with a warning refering to #4563

@chanlito
Copy link

@kevinmarrec What is wrong with this PR?

@kevinmarrec
Copy link
Contributor Author

kevinmarrec commented Dec 16, 2018

@chanlito Check #4563 first commit (d8df0f4), Babel doesn't transpile well Vue SFC.

@vlajos
Copy link

vlajos commented Apr 6, 2019

@kevinmarrec
Should this

// nuxt.config.js
export default {
  build: {
    typescript: true // 🚀
  }
}

still work with 2.6.x ?

@kevinmarrec
Copy link
Contributor Author

@vlajos No, there has been multiple refactors/breaking changes since this behavior, you can check https://nuxtjs.org/guide/typescript to understand properly how you enable TypeScript support in your Nuxt project :)

@vlajos
Copy link

vlajos commented Apr 8, 2019

@kevinmarrec I see. Thanks. I wanted to use it with the nuxtbuilder, but could not really make it working.
I wanted to migrate a nuxt+socket.io project. The basics are roughly like this: https://github.com/nuxt/nuxt.js/tree/dev/examples/with-sockets
Is this possible at all?

@kevinmarrec
Copy link
Contributor Author

@vlajos Hm yes but partially, cause we don't have Nuxt Builder types defined, cause they are depending on too many things in core which also need their types to be defined, which would mean rewrite Nuxt in TypeScript.

If you need further help, join our Discord and I'll try to take some time to give you more guidelines for your migration.

@vlajos
Copy link

vlajos commented Apr 16, 2019

@kevinmarrec Thank you,but practically I gave it up. I wanted to use it mainly to be able to work with typeorm easier, but it seems that it works without typescript as well. Anyway when it becomes a little more supported I may give it another go.
Thanks again!

@danielroe danielroe added the 2.x label Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants