Skip to content

fix(ci): override @astrojs/language-server with an older version #145

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

Merged
merged 12 commits into from
Jul 17, 2024
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
2 changes: 1 addition & 1 deletion docs/demo/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
"@unocss/reset": "^0.59.4",
"astro": "^4.10.3",
"fast-glob": "^3.3.2",
"prettier-plugin-astro": "^0.14.0",
"prettier-plugin-astro": "^0.14.1",
"typescript": "^5.4.5",
"unocss": "^0.59.4"
}
Expand Down
5 changes: 5 additions & 0 deletions docs/tutorialkit.dev/src/components/Layout/Head.astro
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,15 @@ import Default from '@astrojs/starlight/components/Head.astro';
<script is:inline async src="https://www.googletagmanager.com/gtag/js?id=G-64MFE82HG5"></script>

<script is:inline>
// @ts-ignore
window.dataLayer = window.dataLayer || [];

function gtag() {
// @ts-ignore
dataLayer.push(arguments);
}
// @ts-ignore
gtag('js', new Date());
// @ts-ignore
gtag('config', 'G-64MFE82HG5');
</script>
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
"husky": "^9.0.11",
"is-ci": "^3.0.1",
"prettier": "^3.3.2",
"prettier-plugin-astro": "^0.13.0",
"prettier-plugin-astro": "^0.14.1",
"tempfile": "^5.0.0"
},
"lint-staged": {},
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/commands/eject/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ interface PackageJson {
}

const TUTORIALKIT_VERSION = pkg.version;
const REQUIRED_DEPENDENCIES = ['@tutorialkit/runtime', '@webcontainer/api', 'nanostores', '@nanostores/react'];
const REQUIRED_DEPENDENCIES = ['@tutorialkit/runtime', '@webcontainer/api', 'nanostores', '@nanostores/react', 'kleur'];
Copy link
Member Author

Choose a reason for hiding this comment

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

After moving the CLI test from npm to pnpm it revealed a bug in our eject command. 🎉


export function ejectRoutes(flags: Arguments) {
if (flags._[1] === 'help' || flags.help || flags.h) {
Expand Down
8 changes: 1 addition & 7 deletions packages/cli/src/utils/workspace-version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,7 @@ export function updateWorkspaceVersions(
const depVersion = dependencies[dependency];

if (depVersion === 'workspace:*' && filterDependency(dependency)) {
if (process.env.TK_DIRECTORY) {
const name = dependency.split('/')[1];

dependencies[dependency] = `file:${process.env.TK_DIRECTORY}/packages/${name.replace('-', '/')}`;
} else {
dependencies[dependency] = version;
}
dependencies[dependency] = version;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,8 @@ exports[`create and eject a project 1`] = `
"icons/languages/markdown.svg",
"icons/languages/sass.svg",
"icons/languages/ts.svg",
"package-lock.json",
"package.json",
"pnpm-lock.yaml",
"public",
"public/favicon.svg",
"public/logo-dark.svg",
Expand Down
101 changes: 54 additions & 47 deletions packages/cli/tests/create-tutorial.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
import path from 'node:path';
import { execa } from 'execa';
import fs from 'node:fs/promises';
import { tmpdir } from 'node:os';
import path from 'node:path';
import { afterAll, beforeAll, expect, test } from 'vitest';
import { execa } from 'execa';

const tmpDir = path.join(__dirname, '.tmp');
// on CI on windows we want to make sure to use the same drive, so we use a custom logic
const tmpDir =
process.platform === 'win32'
? path.join(path.resolve(__dirname, '../../../..'), '.tmp')
: await fs.mkdtemp(path.join(tmpdir(), 'tk-test-'));
const baseDir = path.resolve(__dirname, '../../..');

const cli = path.join(baseDir, 'packages/cli/dist/index.js');
Expand All @@ -14,7 +19,9 @@ beforeAll(async () => {
});

afterAll(async () => {
await fs.rm(tmpDir, { force: true, recursive: true });
if (process.platform !== 'win32' || !process.env.CI) {
await fs.rm(tmpDir, { force: true, recursive: true });
}
});

test('cannot create project without installing but with starting', async (context) => {
Expand All @@ -23,9 +30,6 @@ test('cannot create project without installing but with starting', async (contex
await expect(
execa('node', [cli, 'create', name, '--no-install', '--start'], {
cwd: tmpDir,
env: {
TK_DIRECTORY: baseDir,
},
}),
).rejects.toThrow('Cannot start project without installing dependencies.');
});
Expand All @@ -36,9 +40,6 @@ test('create a project', async (context) => {

await execa('node', [cli, 'create', name, '--no-install', '--no-git', '--defaults'], {
cwd: tmpDir,
env: {
TK_DIRECTORY: baseDir,
},
});

const projectFiles = await fs.readdir(dest, { recursive: true });
Expand All @@ -50,14 +51,13 @@ test('create and build a project', async (context) => {
const name = context.task.id;
const dest = path.join(tmpDir, name);

await execa('node', [cli, 'create', name, '--no-git', '--no-start', '--defaults'], {
await execa('node', [cli, 'create', name, '--no-git', '--no-install', '--no-start', '--defaults'], {
cwd: tmpDir,
env: {
TK_DIRECTORY: baseDir,
},
});

await execa('npm', ['run', 'build'], {
await runPnpmInstall(dest, baseDir);

await execa('pnpm', ['run', 'build'], {
cwd: dest,
});

Expand All @@ -73,24 +73,25 @@ test('create and eject a project', async (context) => {
const name = context.task.id;
const dest = path.join(tmpDir, name);

await execa('node', [cli, 'create', name, '--no-git', '--no-start', '--defaults'], {
await execa('node', [cli, 'create', name, '--no-git', '--no-install', '--no-start', '--defaults'], {
cwd: tmpDir,
env: {
TK_DIRECTORY: baseDir,
},
});

await runPnpmInstall(dest, baseDir);

await execa('node', [cli, 'eject', name, '--force', '--defaults'], {
cwd: tmpDir,
env: {
TK_DIRECTORY: baseDir,
},
});

// remove `node_modules` before taking the snapshot
await fs.rm(path.join(dest, 'node_modules'), { force: true, recursive: true });
if (process.platform !== 'win32') {
await fs.rm(path.join(dest, 'node_modules'), { force: true, recursive: true, maxRetries: 5 });
}

const projectFiles = await fs.readdir(dest, { recursive: true });
let projectFiles = await fs.readdir(dest, { recursive: true });

if (process.platform === 'win32') {
projectFiles = projectFiles.filter((filePath) => !filePath.startsWith('node_modules'));
}
Comment on lines +86 to +94
Copy link
Member Author

Choose a reason for hiding this comment

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

I really don't like this hack but I couldn't find any other better way to remove this folder on Windows.

It seems that the issue is that when we try to delete the node_modules, we also delete esbuild.exe which get an EBUSY error. If we use the maxRetry property for that usecase, it times out.

My guess is that at this point there's still an esbuild process running. However, I don't know what spawned that esbuild process and I don't want to try to randomly kill all esbuild processes which would be way worse.


expect(projectFiles.map(normaliseSlash).sort()).toMatchSnapshot();
expect(await fs.readFile(path.join(dest, 'astro.config.ts'), 'utf-8')).toMatchSnapshot();
Expand All @@ -100,25 +101,21 @@ test('create, eject and build a project', async (context) => {
const name = context.task.id;
const dest = path.join(tmpDir, name);

await execa('node', [cli, 'create', name, '--no-git', '--no-start', '--defaults'], {
await execa('node', [cli, 'create', name, '--no-git', '--no-install', '--no-start', '--defaults'], {
cwd: tmpDir,
env: {
TK_DIRECTORY: baseDir,
},
});

await runPnpmInstall(dest, baseDir);

await execa('node', [cli, 'eject', name, '--force', '--defaults'], {
cwd: tmpDir,
env: {
TK_DIRECTORY: baseDir,
},
});

await execa('npm', ['install'], {
await execa('pnpm', ['install', '--no-frozen-lockfile'], {
cwd: dest,
});

await execa('npm', ['run', 'build'], {
await execa('pnpm', ['run', 'build'], {
cwd: dest,
});

Expand All @@ -139,9 +136,6 @@ test('cannot eject on an empty folder', async (context) => {
await expect(
execa('node', [cli, 'eject', name, '--force', '--defaults'], {
cwd: tmpDir,
env: {
TK_DIRECTORY: baseDir,
},
}),
).rejects.toThrow('package.json does not exists!');
});
Expand All @@ -157,9 +151,6 @@ test('cannot eject on a node project that is not an Astro project', async (conte
await expect(
execa('node', [cli, 'eject', name, '--force', '--defaults'], {
cwd: tmpDir,
env: {
TK_DIRECTORY: baseDir,
},
}),
).rejects.toThrow('astro.config.ts does not exists!');
});
Expand Down Expand Up @@ -188,9 +179,6 @@ test('cannot eject on an astro project that is not using TutorialKit', async (co
await expect(
execa('node', [cli, 'eject', name, '--force', '--defaults'], {
cwd: tmpDir,
env: {
TK_DIRECTORY: baseDir,
},
}),
).rejects.toThrow(`@tutorialkit${path.sep}astro does not exists!`);
});
Expand All @@ -216,20 +204,39 @@ test('cannot eject on an astro project that is not using TutorialKit 2', async (
`,
);

await execa('npm', ['install'], {
await execa('pnpm', ['install'], {
cwd: dest,
});

await expect(
execa('node', [cli, 'eject', name, '--force', '--defaults'], {
cwd: tmpDir,
env: {
TK_DIRECTORY: baseDir,
},
}),
).rejects.toThrow(`Could not find import to '@tutorialkit/astro'`);
});

function normaliseSlash(filePath: string) {
return filePath.replace(/\\/g, '/');
}

async function runPnpmInstall(dest: string, baseDir: string) {
const packageJsonPath = path.join(dest, 'package.json');
const packageJson = JSON.parse(await fs.readFile(packageJsonPath, 'utf-8'));

packageJson.pnpm = {
overrides: {
'@astrojs/language-server': '2.11.1',
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the version that we were using before that didn't exhibit that issue. I still think that doing the file:.. thing below is wrong. I'm wondering if moving to use .tar.gz files would solve the problem because then their node_modules would be entirely managed by the package manager.

'@tutorialkit/astro': `file:${baseDir}/packages/astro`,
'@tutorialkit/components-react': `file:${baseDir}/packages/components/react`,
'@tutorialkit/runtime': `file:${baseDir}/packages/runtime`,
'@tutorialkit/theme': `file:${baseDir}/packages/theme`,
'@tutorialkit/types': `file:${baseDir}/packages/types`,
},
};

await fs.writeFile(packageJsonPath, JSON.stringify(packageJson, undefined, 2), 'utf8');

await execa('pnpm', ['install'], {
cwd: dest,
});
}
2 changes: 1 addition & 1 deletion packages/template/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"@unocss/reset": "^0.59.4",
"astro": "4.10.3",
"fast-glob": "^3.3.2",
"prettier-plugin-astro": "^0.13.0",
"prettier-plugin-astro": "^0.14.1",
"typescript": "^5.4.5",
"unocss": "^0.59.4"
}
Expand Down
Loading