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

create-astro: always create tsconfig.json #4810

Merged
merged 17 commits into from
Sep 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changeset/many-boats-peel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'create-astro': minor
---

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")
Copy link
Contributor

Choose a reason for hiding this comment

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

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..

Copy link
Member

Choose a reason for hiding this comment

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

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!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

✔ 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.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@
"@octokit/action": "^3.18.1",
"@typescript-eslint/eslint-plugin": "^5.27.1",
"@typescript-eslint/parser": "^5.27.1",
"del": "^6.1.1",
"del": "^7.0.0",
"esbuild": "^0.14.43",
"eslint": "^8.17.0",
"eslint-config-prettier": "^8.5.0",
Expand Down
4 changes: 2 additions & 2 deletions packages/astro/test/benchmark/build.bench.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { performance } from 'perf_hooks';
import { build as astroBuild } from '#astro/build';
import { loadConfig } from '#astro/config';
import { Benchmark } from './benchmark.js';
import del from 'del';
import { deleteAsync } from 'del';
import { Writable } from 'stream';
import { format as utilFormat } from 'util';

Expand Down Expand Up @@ -47,7 +47,7 @@ const benchmarks = [
async setup() {
process.chdir(new URL('../../../../', import.meta.url).pathname);
const spcache = new URL('../../node_modules/.cache/', import.meta.url);
await Promise.all([del(spcache.pathname, { force: true }), setupBuild()]);
await Promise.all([deleteAsync(spcache.pathname, { force: true }), setupBuild()]);
},
run: runBuild,
}),
Expand Down
4 changes: 2 additions & 2 deletions packages/astro/test/benchmark/dev.bench.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import { performance } from 'perf_hooks';
import { Benchmark } from './benchmark.js';
import { runDevServer } from '../helpers.js';
import del from 'del';
import { deleteAsync } from 'del';

const docsExampleRoot = new URL('../../../../docs/', import.meta.url);

Expand Down Expand Up @@ -31,7 +31,7 @@ const benchmarks = [
file: new URL('./dev-server-uncached.json', import.meta.url),
async setup() {
const spcache = new URL('../../node_modules/.cache/', import.meta.url);
await del(spcache.pathname);
await deleteAsync(spcache.pathname);
},
run({ root }) {
return runToStarted(root);
Expand Down
1 change: 1 addition & 0 deletions packages/create-astro/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
"kleur": "^4.1.4",
"ora": "^6.1.0",
"prompts": "^2.4.2",
"strip-ansi": "^7.0.1",
"which-pm-runs": "^1.1.0",
"yargs-parser": "^21.0.1"
},
Expand Down
54 changes: 26 additions & 28 deletions packages/create-astro/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ export async function main() {
choices: [
{
title: 'Relaxed',
value: 'default',
value: 'base',
},
{
title: 'Strict (recommended)',
Expand Down Expand Up @@ -379,42 +379,40 @@ export async function main() {
console.log(` You can safely ignore these files, but don't delete them!`);
console.log(dim(' (ex: tsconfig.json, src/env.d.ts)'));
console.log(``);
tsResponse.typescript = 'default';
tsResponse.typescript = 'base';
await wait(300);
}
if (args.dryRun) {
ora().info(dim(`--dry-run enabled, skipping.`));
} else if (tsResponse.typescript) {
if (tsResponse.typescript !== 'default') {
Copy link
Contributor Author

@mrienstra mrienstra Sep 20, 2022

Choose a reason for hiding this comment

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

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

  1. Removed this line, and its matching closing } (former line 375)
  2. Unindented former lines 347-374 one level (as you'd expect since a wrapping block was removed)

const templateTSConfigPath = path.join(cwd, 'tsconfig.json');
fs.readFile(templateTSConfigPath, (err, data) => {
if (err && err.code === 'ENOENT') {
// 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)
);
const templateTSConfigPath = path.join(cwd, 'tsconfig.json');
fs.readFile(templateTSConfigPath, (err, data) => {
if (err && err.code === 'ENOENT') {
// 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)
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

);

return;
}
return;
}

const templateTSConfig = parse(data.toString());
const templateTSConfig = parse(data.toString());

if (templateTSConfig && typeof templateTSConfig === 'object') {
const result = assign(templateTSConfig, {
extends: `astro/tsconfigs/${tsResponse.typescript}`,
});
if (templateTSConfig && typeof templateTSConfig === 'object') {
const result = assign(templateTSConfig, {
extends: `astro/tsconfigs/${tsResponse.typescript}`,
});

fs.writeFileSync(templateTSConfigPath, stringify(result, null, 2));
} else {
console.log(
yellow(
"There was an error applying the requested TypeScript settings. This could be because the template's tsconfig.json is malformed"
)
);
}
});
}
fs.writeFileSync(templateTSConfigPath, stringify(result, null, 2));
} else {
console.log(
yellow(
"There was an error applying the requested TypeScript settings. This could be because the template's tsconfig.json is malformed"
)
);
}
});
ora().succeed('TypeScript settings applied!');
}

Expand Down
23 changes: 16 additions & 7 deletions packages/create-astro/test/directory-step.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import path from 'path';
import { promises, existsSync } from 'fs';
import { PROMPT_MESSAGES, testDir, setup, promiseWithTimeout, timeout } from './utils.js';
import {
PROMPT_MESSAGES, testDir, setup, promiseWithTimeout, timeout
} from './utils.js';

const inputs = {
nonEmptyDir: './fixtures/select-directory/nonempty-dir',
Expand All @@ -12,19 +14,21 @@ const inputs = {
describe('[create-astro] select directory', function () {
this.timeout(timeout);
it('should prompt for directory when none is provided', function () {
return promiseWithTimeout((resolve) => {
return promiseWithTimeout((resolve, onStdout) => {
const { stdout } = setup();
stdout.on('data', (chunk) => {
onStdout(chunk);
if (chunk.includes(PROMPT_MESSAGES.directory)) {
resolve();
}
});
});
});
it('should NOT proceed on a non-empty directory', function () {
return promiseWithTimeout((resolve) => {
return promiseWithTimeout((resolve, onStdout) => {
const { stdout } = setup([inputs.nonEmptyDir]);
stdout.on('data', (chunk) => {
onStdout(chunk);
if (chunk.includes(PROMPT_MESSAGES.directory)) {
resolve();
}
Expand All @@ -46,34 +50,39 @@ describe('[create-astro] select directory', function () {
if (!existsSync(resolvedEmptyDirPath)) {
await promises.mkdir(resolvedEmptyDirPath);
}
return promiseWithTimeout((resolve) => {
return promiseWithTimeout((resolve, onStdout) => {
const { stdout } = setup([inputs.emptyDir]);
stdout.on('data', (chunk) => {
onStdout(chunk);
if (chunk.includes(PROMPT_MESSAGES.template)) {
resolve();
}
});
});
});
it('should proceed when directory does not exist', function () {
return promiseWithTimeout((resolve) => {
return promiseWithTimeout((resolve, onStdout) => {
const { stdout } = setup([inputs.nonexistentDir]);
stdout.on('data', (chunk) => {
onStdout(chunk);
if (chunk.includes(PROMPT_MESSAGES.template)) {
resolve();
}
});
});
});
it('should error on bad directory selection in prompt', function () {
return promiseWithTimeout((resolve) => {
return promiseWithTimeout((resolve, onStdout) => {
let wrote = false;
const { stdout, stdin } = setup();
stdout.on('data', (chunk) => {
onStdout(chunk);
if (chunk.includes('is not empty!')) {
resolve();
}
if (chunk.includes(PROMPT_MESSAGES.directory)) {
if (!wrote && chunk.includes(PROMPT_MESSAGES.directory)) {
stdin.write(`${inputs.nonEmptyDir}\x0D`);
wrote = true;
}
});
});
Expand Down
117 changes: 117 additions & 0 deletions packages/create-astro/test/typescript-step.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
import { expect } from 'chai';
import { deleteSync } from 'del';
import { existsSync, mkdirSync, readdirSync, readFileSync } from 'fs';
import path from 'path';
import {
PROMPT_MESSAGES, testDir, setup, promiseWithTimeout, timeout
} from './utils.js';

const inputs = {
emptyDir: './fixtures/select-typescript/empty-dir',
};

function isEmpty(dirPath) {
return !existsSync(dirPath) || readdirSync(dirPath).length === 0;
}

function ensureEmptyDir() {
const dirPath = path.resolve(testDir, inputs.emptyDir);
if (!existsSync(dirPath)) {
mkdirSync(dirPath, { recursive: true });
} else if (!isEmpty(dirPath)) {
const globPath = path.resolve(dirPath, '*');
deleteSync(globPath, { dot: true });
}
}

function getTsConfig(installDir) {
const filePath = path.resolve(testDir, installDir, 'tsconfig.json');
return JSON.parse(readFileSync(filePath, 'utf-8'));
}

describe('[create-astro] select typescript', function () {
this.timeout(timeout);

beforeEach(ensureEmptyDir);

afterEach(ensureEmptyDir);

it('should prompt for typescript when none is provided', async function () {
return promiseWithTimeout((resolve, onStdout) => {
const { stdout } = setup([
inputs.emptyDir,
'--template', 'minimal',
'--install', '0',
'--git', '0'
]);
stdout.on('data', (chunk) => {
onStdout(chunk);
if (chunk.includes(PROMPT_MESSAGES.typescript)) {
resolve();
}
});
}, () => lastStdout);
});

it('should not prompt for typescript when provided', async function () {
return promiseWithTimeout((resolve, onStdout) => {
const { stdout } = setup([
inputs.emptyDir,
'--template', 'minimal',
'--install', '0',
'--git', '0',
'--typescript', 'base'
]);
stdout.on('data', (chunk) => {
onStdout(chunk);
if (chunk.includes(PROMPT_MESSAGES.typescriptSucceed)) {
resolve();
}
});
}, () => lastStdout);
});

it('should use "strict" config when specified', async function () {
return promiseWithTimeout((resolve, onStdout) => {
let wrote = false;
const { stdout, stdin } = setup([
inputs.emptyDir,
'--template', 'minimal',
'--install', '0',
'--git', '0'
]);
stdout.on('data', (chunk) => {
onStdout(chunk);
if (!wrote && chunk.includes(PROMPT_MESSAGES.typescript)) {
stdin.write('\x1B\x5B\x42\x0D');
wrote = true;
}
if (chunk.includes(PROMPT_MESSAGES.typescriptSucceed)) {
const tsConfigJson = getTsConfig(inputs.emptyDir);
expect(tsConfigJson).to.deep.equal({'extends': 'astro/tsconfigs/strict'});
resolve();
}
});
}, () => lastStdout);
});

it('should create tsconfig.json when missing', async function () {
return promiseWithTimeout((resolve, onStdout) => {
const { stdout } = setup([
inputs.emptyDir,
'--template', 'cassidoo/shopify-react-astro',
'--install', '0',
'--git', '0',
'--typescript', 'base'
]);
stdout.on('data', (chunk) => {
onStdout(chunk);
if (chunk.includes(PROMPT_MESSAGES.typescriptSucceed)) {
const tsConfigJson = getTsConfig(inputs.emptyDir);
expect(tsConfigJson).to.deep.equal({'extends': 'astro/tsconfigs/base'});
resolve();
}
});
}, () => lastStdout);
});
});
26 changes: 20 additions & 6 deletions packages/create-astro/test/utils.js
Original file line number Diff line number Diff line change
@@ -1,31 +1,45 @@
import { execa } from 'execa';
import { fileURLToPath } from 'url';
import { dirname } from 'path';
import stripAnsi from 'strip-ansi';
import { fileURLToPath } from 'url';

const __filename = fileURLToPath(import.meta.url);
export const testDir = dirname(__filename);
export const timeout = 5000;

const createAstroError = new Error(
'Timed out waiting for create-astro to respond with expected output.'
);
const timeoutError = function (details) {
let errorMsg =
'Timed out waiting for create-astro to respond with expected output.';
if (details) {
errorMsg += '\nLast output: "' + details + '"';
}
return new Error(errorMsg);
}

export function promiseWithTimeout(testFn) {
return new Promise((resolve, reject) => {
let lastStdout;
function onStdout (chunk) {
lastStdout = stripAnsi(chunk.toString()).trim() || lastStdout;
}

const timeoutEvent = setTimeout(() => {
reject(createAstroError);
reject(timeoutError(lastStdout));
}, timeout);
function resolver() {
clearTimeout(timeoutEvent);
resolve();
}
testFn(resolver);

testFn(resolver, onStdout);
});
}

export const PROMPT_MESSAGES = {
directory: 'Where would you like to create your new project?',
template: 'Which template would you like to use?',
typescript: 'How would you like to setup TypeScript?',
typescriptSucceed: 'Next steps'
};

export function setup(args = []) {
Expand Down
Loading