`create-astro`: always create `tsconfig.json` #4810

mrienstra · 2022-09-20T09:18:59Z

Changes

Alway write chosen config to tsconfig.json.

Before: Only when strict or strictest was selected
After: Also when base is selected (via "Relaxed" or "I prefer not to use TypeScript")

Testing

Added new typescript-step.test.js test file to test existing and new behavior related to the typescript prompt / --typescript CLI argument).

Docs

Existing create-astro docs don't mention tsconfig.json, so no changes are needed.

Motivation

Before this PR:

If base:
A) Do nothing: assumes a default base tsconfig.json, which is the case for all 23 official templates, but not necessarily for third-party templates.
If strict or strictest:
A) Make sure tsconfig.json exists (create if missing).
B) Write chosen extends to tsconfig.json (e.g. { "extends": "astro/tsconfigs/strict" }).

After this PR: base is treated the same as strict or strictest.

Template without `tsconfig.json`

The example command for installing a third-party template -- rather conveniently for the sake of this PR -- uses a template without a tsconfig.json file, and installing it (prior to the changes in this PR) with the base Typescript option results in no tsconfig.json file, rather than a tsconfig.json file containing { "extends": "astro/tsconfigs/base" } as would be expected.

Template with `tsconfig.json` not containing `{ "extends": "astro/tsconfigs/base" }`

If a third-party template doesn't use the base config, installing it (prior to the changes in this PR) with the base Typescript option results in no changes to tsconfig.json, rather than setting extends to astro/tsconfigs/base as would be expected.

Currently, we only make sure `tsconfig.json` exists when `strict` or `strictest` is selected. Both `default` & `optout` are intended to correspond to `base` -- and will do so for all [23 official templates](https://github.com/withastro/astro/tree/main/examples), but not necessarily for third-party templates. The [example command for installing a third-party template](https://github.com/withastro/astro/blob/a800bf7/packages/create-astro/README.md?plain=1#L31-L35) is (rather conveniently for the sake of this PR!) an example of a template without a `tsconfig.json` file, and installing it with the `default` ("Relaxed") Typescript option results in no `tsconfig.json` file, rather than a `tsconfig.json` file containing `{ "extends": "astro/tsconfigs/base" }` as would be expected. This PR addresses this scenario. It also explicitly sets the `tsconfig.json` file to `{ "extends": "astro/tsconfigs/base" }` when `default` (which I renamed to `base`, still presented to the user as "Relaxed") or `optout` is selected (`optout` has always printed a warning about the importance of `tsconfig.json` & `src/env.d.ts` but otherwise behaved identically to `default`). This is necessary in two scenarios: 1. When the `tsconfig.json` file was created by this script. 2. When it either didn't already include `"extends"`, or it extended a different config by default. For example, some third-party templates might default to `strict`, in which case I'm guessing we'd want to respect the user's choice and change that to `base`.

(without this it triggers many times)

changeset-bot · 2022-09-20T09:19:03Z

🦋 Changeset detected

Latest commit: fe6ba76

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
create-astro	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

mrienstra · 2022-09-20T09:22:10Z

scripts/cmd/build.js

@@ -109,5 +109,5 @@ export default async function build(...args) {
 }

 async function clean(outdir) {
-	return del([`${outdir}/**`, `!${outdir}/**/*.d.ts`]);
+	return deleteAsync([`${outdir}/**`, `!${outdir}/**/*.d.ts`]);


Not sure if this is relevant here: https://github.com/sindresorhus/del#beware

Doesn't appear to be a breaking change from prior version: https://github.com/sindresorhus/del/tree/v6.1.1#beware

... So it should be behaving the same as it was before this PR.

(see if this fixes failing tests)

(since I'm about to trigger another CI run by pushing a commit, might as well try this too)

Typescript tests are slower than directory tests, but they are all usually less than 5000 ms. Less complexity, easier to maintain.

mrienstra · 2022-09-20T19:05:14Z

packages/create-astro/src/index.ts

 		await wait(300);
 	}
 	if (args.dryRun) {
 		ora().info(dim(`--dry-run enabled, skipping.`));
 	} else if (tsResponse.typescript) {
-		if (tsResponse.typescript !== 'default') {


The diff GitHub is showing here is unnecessarily complex, the only differences from here down are:

Removed this line, and its matching closing } (former line 375)

Unindented former lines 347-374 one level (as you'd expect since a wrapping block was removed)

jasikpark · 2022-09-21T17:20:22Z

.changeset/many-boats-peel.md

+Alway write chosen config to `tsconfig.json`.
+
+- Before: Only when `strict` & `strictest` was selected
+- After: Also when `base` is selected (via "Relaxed" or "I prefer not to use TypeScript")


I wonder whether users would be confused as to why they have an extra file in their project when they chose "no typescript"? I suppose they can just delete it later..

I wonder whether users would be confused as to why they have an extra file in their project when they chose "no typescript"? I suppose they can just delete it later..

We're planning to update the message soon so it's more clear!

Existing behavior (before this PR):
When you select "I prefer not to use Typescript", it prints this:

✔ How would you like to setup TypeScript? › I prefer not to use TypeScript

⚠ Astro ❤️ TypeScript!
Astro supports TypeScript inside of ".astro" component scripts, so
we still need to create some TypeScript-related files in your project.
You can safely ignore these files, but don't delete them!
(ex: tsconfig.json, src/env.d.ts)

✔ TypeScript settings applied!

Also, before this PR, those files would always be there when using the official templates, they would only be missing if you used a third-party template that didn't include them.

jasikpark · 2022-09-21T17:23:25Z

packages/create-astro/src/index.ts

+				// If the template doesn't have a tsconfig.json, let's add one instead
+				fs.writeFileSync(
+					templateTSConfigPath,
+					stringify({ extends: `astro/tsconfigs/${tsResponse.typescript}` }, null, 2)


wonder if it's helpful linking to the docs for this in a comment as well?
https://docs.astro.build/en/guides/typescript/#setup

Not a bad idea! I didn't change those lines (just dedented them), so doesn't necessarily need to be added in this PR, but might as well toss it in, feel free to add a commit for that.

Princesseuh

Thank you for this PR! The tests are notably lovely. I'm a bit thorn on if adding a tsconfig to third-party template is good or not, for the following reason:

The template might be a monorepo, which shouldn't have a tsconfig.json with our extend in a lot of cases
The template might be expecting specific tsconfig.json settings

Maybe for third-party templates that do have a tsconfig.json we could add another prompt option with something like "Use the template's settings".

HOWEVER, despite that, I think this is overall an improvement and since it's a fairly niche use case to use third party templates, we can change this in the future if it turns out to be a problem. Thank you again for this PR!

mrienstra · 2022-09-21T20:35:54Z

The template might be a monorepo, which shouldn't have a tsconfig.json with our extend in a lot of cases

The template might be expecting specific tsconfig.json settings

Maybe for third-party templates that do have a tsconfig.json we could add another prompt option with something like "Use the template's settings".

Excellent points, we could add a warning to the docs, or leave an issue open?

mrienstra · 2022-09-21T20:37:22Z

Huh, weird, CI / Lint is failing on unrelated code... 🤔

Princesseuh · 2022-09-21T20:42:44Z

Huh, weird, CI / Lint is failing on unrelated code... 🤔

Yeah, there's a bug in our CI pipeline that made this go unnoticed from another PR. No worries, it's unrelated to your changes!

mrienstra added 5 commits September 19, 2022 14:19

update del 6.1.1 --> 7.0.0

d402ea5

test: prevent excess writes

25aeb9b

(without this it triggers many times)

test: create-astro typescript prompt

534472c

changeset

d26485f

github-actions bot added pkg: astro Related to the core `astro` package (scope) pkg: create-astro Related to the `create-astro` package (scope) labels Sep 20, 2022

mrienstra commented Sep 20, 2022

View reviewed changes

mrienstra added 7 commits September 20, 2022 02:51

fix: recursive mkdirSync

193b01e

test: longer timeout for windows-latest OS

1aa2189

(see if this fixes failing tests)

better glob path creation, don't hardcode /

bda52db

test: longer timeout for windows-latest OS

c054cef

(since I'm about to trigger another CI run by pushing a commit, might as well try this too)

create-astro test: show last CLI output on timeout

3b36ad5

drop variable timeout

5b27c95

Typescript tests are slower than directory tests, but they are all usually less than 5000 ms. Less complexity, easier to maintain.

DRY new error output

fbec5e5

mrienstra commented Sep 20, 2022

View reviewed changes

jasikpark reviewed Sep 21, 2022

View reviewed changes

Princesseuh approved these changes Sep 21, 2022

View reviewed changes

Princesseuh added 3 commits September 21, 2022 15:56

Merge remote-tracking branch 'upstream/main' into patch-1

fd17e9a

Update lockfile

24a06ac

Sync lockfile with main

305576c

Princesseuh added 2 commits September 22, 2022 14:06

Merge remote-tracking branch 'upstream/main' into patch-1

ddd9723

Update lockfile

fe6ba76

matthewp merged commit 7481ffd into withastro:main Sep 22, 2022

astrobot-houston mentioned this pull request Sep 22, 2022

[ci] release #4846

Merged

mrienstra mentioned this pull request Oct 26, 2022

dev: identifying flaky tests? #5195

Closed

mrienstra mentioned this pull request Nov 3, 2022

create-astro does not work #5227

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`create-astro`: always create `tsconfig.json` #4810

`create-astro`: always create `tsconfig.json` #4810

mrienstra commented Sep 20, 2022

changeset-bot bot commented Sep 20, 2022 •

edited

Loading

mrienstra Sep 20, 2022

mrienstra Sep 20, 2022 •

edited

Loading

jasikpark Sep 21, 2022

Princesseuh Sep 21, 2022

mrienstra Sep 21, 2022

jasikpark Sep 21, 2022

mrienstra Sep 21, 2022

Princesseuh left a comment

mrienstra commented Sep 21, 2022

mrienstra commented Sep 21, 2022

Princesseuh commented Sep 21, 2022

create-astro: always create tsconfig.json #4810

create-astro: always create tsconfig.json #4810

Conversation

mrienstra commented Sep 20, 2022

Changes

Testing

Docs

Motivation

Template without tsconfig.json

Template with tsconfig.json not containing { "extends": "astro/tsconfigs/base" }

changeset-bot bot commented Sep 20, 2022 • edited Loading

🦋 Changeset detected

mrienstra Sep 20, 2022

Choose a reason for hiding this comment

mrienstra Sep 20, 2022 • edited Loading

Choose a reason for hiding this comment

jasikpark Sep 21, 2022

Choose a reason for hiding this comment

Princesseuh Sep 21, 2022

Choose a reason for hiding this comment

mrienstra Sep 21, 2022

Choose a reason for hiding this comment

jasikpark Sep 21, 2022

Choose a reason for hiding this comment

mrienstra Sep 21, 2022

Choose a reason for hiding this comment

Princesseuh left a comment

Choose a reason for hiding this comment

mrienstra commented Sep 21, 2022

mrienstra commented Sep 21, 2022

Princesseuh commented Sep 21, 2022

`create-astro`: always create `tsconfig.json` #4810

`create-astro`: always create `tsconfig.json` #4810

Template without `tsconfig.json`

Template with `tsconfig.json` not containing `{ "extends": "astro/tsconfigs/base" }`

changeset-bot bot commented Sep 20, 2022 •

edited

Loading

mrienstra Sep 20, 2022 •

edited

Loading