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(create-app): add 'vanilla-ts' template #2849

Closed
wants to merge 1 commit into from

Conversation

mrienstra
Copy link

@mrienstra mrienstra commented Apr 3, 2021

Description

Adds a 'vanilla-ts' template to @vitejs/create-app.

Additional context

After experimenting with this template locally, I discovered a duplicate PR, #2814. I began reviewing it (and subsequently finished reviewing it), but decided it would be slightly more straightforward to open this PR.

I copied main.js from the vanilla template and renamed it main.ts. I had to make a small change to satisfy Typescript, as the original version:

document.querySelector('#app').innerHTML = `

... Throws an Object is possibly 'null' error.

I looked at all of the existing packages/create-app/template-*/tsconfig.json files, and came up with a tsconfig.json for this template which (hopefully) represents a reasonable starting point. I opted to not specify lib, and instead allow it to be set by target (which is also the case in the template-svelte-ts tsconfig.json).


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate. (See note above)
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@Shinigami92 Shinigami92 self-requested a review April 3, 2021 23:10
@Shinigami92 Shinigami92 added enhancement New feature or request p2-nice-to-have Not breaking anything but nice to have (priority) labels Apr 3, 2021
"compilerOptions": {
"target": "ESNext",
"module": "ESNext",
"moduleResolution": "node",
Copy link
Member

@antfu antfu Apr 4, 2021

Choose a reason for hiding this comment

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

Think we should add "lib": ["esnext", "dom"],. It should not conflict with target I think

Copy link
Author

@mrienstra mrienstra Apr 4, 2021

Choose a reason for hiding this comment

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

I'm fine with that!

To the best of my knowledge (just opened a PR on the subject, so should have confirmation soon): Dropping "lib" results in a default value being used, controlled by "target" -- in this case, since "target" is "ESNext", "lib" will be set to "ESNext.Full" -- which cannot be set directly.

"ESNext.Full" is (currently, it's a moving target) the equivalent of:
["esnext", "dom", "webworker.importscripts", "scripthost", "dom.iterable"]

But perhaps using "lib" is friendlier, since the behavior is more obvious?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think specifying lib could be more explicit. Where we also have it for Vue TS

Copy link
Author

Choose a reason for hiding this comment

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

@antfu, yup, all of the template tsconfig.json have "lib", except for for the template-svelte-ts tsconfig.json, been considering floating the idea of making them a little more consistent. Obviously some of the differences are necessary, but others seem arbitrary.

@patak-dev
Copy link
Member

Just for reference, a PR to add a vanilla-ts was not merged #2023 (comment), on the basis that it was just adding a config file over the js equivalent. But this is something that several people in the community have been requesting. I think there is merit in having this template in create-app

@antfu
Copy link
Member

antfu commented Apr 4, 2021

As #2023 comes first, let's work on it and have discussions there instead. Thanks for your contributions.

@antfu antfu closed this Apr 4, 2021
@mrienstra
Copy link
Author

As #2023 comes first, let's work on it and have discussions there instead. Thanks for your contributions.

Sounds good, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request p2-nice-to-have Not breaking anything but nice to have (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants