Skip to content
This repository was archived by the owner on Mar 4, 2025. It is now read-only.

features selection #152

Closed
wants to merge 8 commits into from
Closed

Conversation

sascha245
Copy link
Contributor

@sascha245 sascha245 commented Dec 15, 2018

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

Add features selection similar to vue-cli 3:

  • Typescript
  • PWA
  • Linter (if selected, eslint by default and tslint if typescript is selected)
  • Prettier

Solves #23 and in some parts #42

@sascha245 sascha245 changed the title Feature/nuxt modules add features choice Dec 15, 2018
@sascha245 sascha245 changed the title add features choice features selection Dec 15, 2018
@atinux
Copy link
Member

atinux commented Dec 19, 2018

Hi @sascha245

Thanks for the PR, I am waiting for others approval for the TS part.

But it's looking great so far for me and like the checkboxes option :)

Copy link
Member

@atinux atinux left a comment

Choose a reason for hiding this comment

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

Waiting for Kevin and Hartmut approvals or requested changes for the TS part.

@aldarund
Copy link
Collaborator

LGTM

Copy link

@hartmut-co-uk hartmut-co-uk left a comment

Choose a reason for hiding this comment

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

template/nuxt/nuxt.config.js
needs to enable the typescript flag for build config:

build: {
  typescript: true
}

@hartmut-co-uk
Copy link

@hartmut-co-uk
Copy link

I think generally it would be better / necessary to patiently await one more @kevinmarrec iteration which will bring further improvements around ts-loader and fork-ts-checker-webpack-plugin + typescript nuxt configuration - to finalise the typescript part of this PR.

@kevinmarrec
Copy link
Contributor

kevinmarrec commented Dec 19, 2018

Alright, first, thanks @sascha245 for the PR !

I suggest indeed (as @hartmut-co-uk said) to wait for my next iteration which will likely change things around this PR, so that's better IMO to wait and then I'll review it before we got this merged :).

This PR uses nuxt-ts and nuxt-tslint packages which will be replaced by the ts-loader + fork-ts-checker-webpack-plugin combination.

@sascha245
Copy link
Contributor Author

Yep, in that case it would indeed be better to wait.
Tell me when the new version is out so that I can make the necessary changes ;-)

@hartmut-co-uk
Copy link

One other option might be to strip everything ts+tslint related from this PR so it can be merged already?
It's already combining and bringing in multiple great features like 'checkboxes' & PWA, eslint+prettier.

It's going to be a lot easier and more transparent to add the TS support separately afterwards.

@kevinmarrec has created the additional 2 PRs now:
nuxt/nuxt#4611
nuxt/nuxt#4613

@kevinmarrec
Copy link
Contributor

kevinmarrec commented Dec 22, 2018

I agree with @hartmut-co-uk that it would be easier to add the TS options on create-nuxt-app on a separate PR, so that other options (PWA, eslint + prettier) get merged directly.

@sascha245
Copy link
Contributor Author

Note that I didn't implement eslint and prettier myself, I just transformed the separated yes/no questions to checkboxes.

Anyway, I created a new and clean pull request (#156) containing only pwa, eslint, prettier and axios checkboxes.
I will add TS and TSLint support once again in a separate PR when the new Nuxt version is out.
Hope that makes things easier for you.

With this, I will close this pull request.

@sascha245 sascha245 closed this Dec 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants