Skip to content

feat: tutorialkit eject command #81

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 31 commits into from
Jul 4, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
f68571f
feat: `tutorialkit eject` command
Nemikolh Jun 19, 2024
4513bfc
fix: bugs with updateProperties and boolean values
Nemikolh Jun 19, 2024
f1b66af
fix: simplify a bit enterprise.ts
Nemikolh Jun 19, 2024
3228705
fix: rename to avoid the collision
Nemikolh Jun 19, 2024
98740da
Apply suggestions from code review
Nemikolh Jun 20, 2024
bc536b9
Merge branch 'main' into joan/eject-command
SamVerschueren Jun 20, 2024
e303332
chore: add tests for eject command
Nemikolh Jun 20, 2024
ce68177
fix: make sure to only update our stuff and not users dependencies
Nemikolh Jun 20, 2024
2047b0d
Merge branch 'main' into joan/eject-command
Nemikolh Jun 20, 2024
72bffb2
Merge branch 'main' into joan/eject-command
Nemikolh Jun 20, 2024
e374027
fix: update snapshot after merge
Nemikolh Jun 20, 2024
17d194c
fix: add more tests
Nemikolh Jun 21, 2024
fe1211d
fix: test on Windows
Nemikolh Jun 24, 2024
0a8b9dd
Apply suggestions from code review
Nemikolh Jun 26, 2024
b3a8135
Merge remote-tracking branch 'origin/main' into joan/eject-command
Nemikolh Jun 26, 2024
499dd36
fix: lint issues
Nemikolh Jun 26, 2024
f00eeee
fix: add test snapshot for astro config after eject
Nemikolh Jun 26, 2024
ddb7ab6
fix: code review
Nemikolh Jun 26, 2024
fc2bde2
fix: remove only
Nemikolh Jun 26, 2024
e8a1f7a
fix: lint error
Nemikolh Jun 26, 2024
9fd7ee0
Merge branch 'main' into joan/eject-command
SamVerschueren Jul 2, 2024
4013b62
fix: test
Nemikolh Jul 2, 2024
a1f1aff
fix: code review
Nemikolh Jul 2, 2024
f5ca361
fix: update tests snapshots
Nemikolh Jul 2, 2024
75925a8
Merge branch 'main' into joan/eject-command
Nemikolh Jul 3, 2024
3cafab6
Merge branch 'main' into joan/eject-command
Nemikolh Jul 3, 2024
1033feb
fix: update snapshot
Nemikolh Jul 3, 2024
be68c98
Merge branch 'main' into joan/eject-command
Nemikolh Jul 3, 2024
c03d28e
Update packages/cli/src/commands/eject/index.ts
Nemikolh Jul 4, 2024
0e2b6d8
fix: code review replace preferred-pm with which-pm and add color to …
Nemikolh Jul 4, 2024
a52d0bc
Merge branch 'main' into joan/eject-command
Nemikolh Jul 4, 2024
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 eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export default [
},
}),
{
files: ['**/env.d.ts'],
files: ['**/env.d.ts', '**/env-default.d.ts'],
rules: {
'@typescript-eslint/triple-slash-reference': 'off',

Expand Down
11 changes: 2 additions & 9 deletions packages/cli/src/commands/create/enterprise.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import fs from 'node:fs';
import path from 'node:path';
import { parseAstroConfig, replaceArgs } from './astro-config.js';
import { generate } from './babel.js';
import { generateAstroConfig, parseAstroConfig, replaceArgs } from '../../utils/astro-config.js';
import type { CreateOptions } from './options.js';

export async function setupEnterpriseConfig(dest: string, flags: CreateOptions) {
Expand Down Expand Up @@ -38,13 +37,7 @@ export async function setupEnterpriseConfig(dest: string, flags: CreateOptions)
astroConfig,
);

const defaultExport = 'export default defineConfig';
let output = generate(astroConfig);

// add a new line
output = output.replace(defaultExport, `\n${defaultExport}`);

fs.writeFileSync(configPath, output);
fs.writeFileSync(configPath, generateAstroConfig(astroConfig));
}
}

Expand Down
17 changes: 1 addition & 16 deletions packages/cli/src/commands/create/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { pkg } from '../../pkg.js';
import { errorLabel, primaryLabel, printHelp, warnLabel } from '../../utils/messages.js';
import { generateProjectName } from '../../utils/project.js';
import { assertNotCanceled } from '../../utils/tasks.js';
import { updateWorkspaceVersions } from '../../utils/workspace-version.js';
import { setupEnterpriseConfig } from './enterprise.js';
import { initGitRepo } from './git.js';
import { installAndStart } from './install-start.js';
Expand Down Expand Up @@ -264,22 +265,6 @@ function updatePackageJson(dest: string, projectName: string, flags: CreateOptio
fs.writeFileSync(pkgPath, JSON.stringify(pkgJson, undefined, 2));
}

function updateWorkspaceVersions(dependencies: Record<string, string>, version: string) {
for (const dependency in dependencies) {
const depVersion = dependencies[dependency];

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

dependencies[dependency] = `file:${process.env.TK_DIRECTORY}/packages/${name.replace('-', '/')}`;
} else {
dependencies[dependency] = version;
}
}
}
}

function updateReadme(dest: string, packageManager: PackageManager, flags: CreateOptions) {
if (flags.dryRun) {
return;
Expand Down
177 changes: 177 additions & 0 deletions packages/cli/src/commands/eject/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
import chalk from 'chalk';
import fs from 'node:fs';
import path from 'node:path';
import type { Arguments } from 'yargs-parser';
import { pkg } from '../../pkg.js';
import { DEFAULT_VALUES, type EjectOptions } from './options.js';
import { errorLabel, printHelp, primaryLabel } from '../../utils/messages.js';
import { parseAstroConfig, replaceArgs, generateAstroConfig } from '../../utils/astro-config.js';
import { updateWorkspaceVersions } from '../../utils/workspace-version.js';

interface PackageJson {
dependencies: Record<string, string>;
devDependencies: Record<string, string>;
}

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

export function ejectRoutes(flags: Arguments) {
if (flags._[1] === 'help' || flags.help || flags.h) {
printHelp({
commandName: `${pkg.name} eject`,
usage: '[folder] [...options]',
tables: {
Options: [
[
'--force',
`Overwrite existing files in the target directory without prompting (default ${chalk.yellow(DEFAULT_VALUES.force)})`,
],
],
},
});

return 0;
}

try {
return _eject(flags);
} catch (error) {
console.error(`${errorLabel()} Command failed`);

if (error.stack) {
console.error(`\n${error.stack}`);
}

process.exit(1);
}
}

async function _eject(flags: EjectOptions) {
let folderPath = flags._[1] !== undefined ? String(flags._[1]) : undefined;

if (folderPath === undefined) {
folderPath = process.cwd();
} else {
folderPath = path.resolve(process.cwd(), folderPath);
}

/**
* First we make sure that the destination has the correct files
* and that there won't be any files overwritten in the process.
*
* If there are any and `force` was not specified we abort.
*/
const { astroConfigPath, srcPath, pkgJsonPath, astroIntegrationPath, srcDestPath } = validateDestination(
folderPath,
flags.force,
);

/**
* We proceed with the astro configuration.
*
* There we must disable the default routes so that the
* new routes that we're copying will be automatically picked up.
*/
const astroConfig = await parseAstroConfig(astroConfigPath);

replaceArgs({ defaultRoutes: false }, astroConfig);

fs.writeFileSync(astroConfigPath, generateAstroConfig(astroConfig));

// we copy all assets from the `default` folder into the `src` folder
fs.cpSync(srcPath, srcDestPath, { recursive: true });

/**
* Last, we ensure that the `package.json` contains the extra dependencies.
* If any are missing we suggest to install the new dependencies.
*/
const pkgJson: PackageJson = JSON.parse(fs.readFileSync(pkgJsonPath, 'utf-8'));

const astroIntegrationPkgJson: PackageJson = JSON.parse(
fs.readFileSync(path.join(astroIntegrationPath, 'package.json'), 'utf-8'),
);

const newDependencies = [];

for (const dep of REQUIRED_DEPENDENCIES) {
if (!(dep in pkgJson.dependencies) && !(dep in pkgJson.devDependencies)) {
pkgJson.dependencies[dep] = astroIntegrationPkgJson.dependencies[dep];
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe not super important for now, but we might want to check the version of the dependency and warn the user that they might need to upgrade/downgrade or something.

Copy link
Member Author

Choose a reason for hiding this comment

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

We do! The let hasChanged = false; is used for that purpose.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm so technically this can result in false positives if say I have nanostores in my package.json already but the range just doesn't match but the version would be the same, e.g.

^1.1.1 !== 1.1.1

Given that there's no new patch version and 1.1.1 is the latest. Wouldn't it be better to check node_modules or check the lock file? But maybe this is good enough and a warning is good in any case which would force the user to double check the dependency.

Copy link
Member Author

Choose a reason for hiding this comment

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

If they have an incompatible version of nanostores, there's not much we can do as they'll have to update the code they've just ejected. Hopefully there's a TypeScript error so that they know.

If it's compatible, then this code uses their version, so I think this code is good?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea I think it's fine cause we only show the warning if that dependency is missing entirely in the package.json. I was just wondering if we can be smarter here and maybe check the version and print a hint if the version doesn't match to tell them that there may be incompatibilities.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah good point! Oh yeah that would be really nice actually.

Copy link
Contributor

Choose a reason for hiding this comment

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

One possible way would be to leverage npm why --json to see what version is installed and what version we require.


newDependencies.push(dep);
}
}

updateWorkspaceVersions(pkgJson.dependencies, TUTORIALKIT_VERSION, (dependency) =>
REQUIRED_DEPENDENCIES.includes(dependency),
);

if (newDependencies.length > 0) {
fs.writeFileSync(pkgJsonPath, JSON.stringify(pkgJson, undefined, 2), { encoding: 'utf-8' });

console.log(
primaryLabel('INFO'),
`New dependencies added: ${newDependencies.join(', ')}. Install the new dependencies before proceeding`,
);
}
}

function validateDestination(folder: string, force: boolean) {
assertExists(folder);

const pkgJsonPath = assertExists(path.join(folder, 'package.json'));
const astroConfigPath = assertExists(path.join(folder, 'astro.config.ts'));
const srcDestPath = assertExists(path.join(folder, 'src'));

const astroIntegrationPath = assertExists(path.resolve(folder, 'node_modules', '@tutorialkit', 'astro'));

const srcPath = path.join(astroIntegrationPath, 'dist', 'default');

// check that there are no collision
if (!force) {
walk(srcPath, (relativePath) => {
const destination = path.join(srcDestPath, relativePath);

if (fs.existsSync(destination)) {
throw new Error(
`Eject aborted because '${destination}' would be overwritten by this command. Use ${chalk.yellow('--force')} to ignore this error.`,
);
}
});
}

return {
astroConfigPath,
astroIntegrationPath,
pkgJsonPath,
srcPath,
srcDestPath,
};
}

function assertExists(filePath: string) {
if (!fs.existsSync(filePath)) {
throw new Error(`${filePath} does not exists!`);
}

return filePath;
}

function walk(root: string, visit: (relativeFilePath: string) => void) {
function traverse(folder: string, pathPrefix: string) {
for (const filename of fs.readdirSync(folder)) {
const filePath = path.join(folder, filename);
const stat = fs.statSync(filePath);

const relativeFilePath = path.join(pathPrefix, filename);

if (stat.isDirectory()) {
traverse(filePath, relativeFilePath);
} else {
visit(relativeFilePath);
}
}
}

traverse(root, '');
}
8 changes: 8 additions & 0 deletions packages/cli/src/commands/eject/options.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export interface EjectOptions {
_: Array<string | number>;
force?: boolean;
}

export const DEFAULT_VALUES = {
force: false,
};
8 changes: 6 additions & 2 deletions packages/cli/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@
import chalk from 'chalk';
import yargs from 'yargs-parser';
import { createTutorial } from './commands/create/index.js';
import { ejectRoutes } from './commands/eject/index.js';
import { pkg } from './pkg.js';
import { errorLabel, primaryLabel, printHelp } from './utils/messages.js';

type CLICommand = 'version' | 'help' | 'create';
type CLICommand = 'version' | 'help' | 'create' | 'eject';

const supportedCommands = new Set(['version', 'help', 'create']);
const supportedCommands = new Set<string>(['version', 'help', 'create', 'eject'] satisfies CLICommand[]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we just type this as Set<CLICommand>?

Copy link
Member Author

Choose a reason for hiding this comment

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

We can't because then we get a type error below when trying to do supportedCommands.has(string).

The reason I made that change was to make sure the values are checked against the CLICommand type.


cli();

Expand Down Expand Up @@ -53,6 +54,9 @@ async function runCommand(cmd: CLICommand, flags: yargs.Arguments): Promise<numb
case 'create': {
return createTutorial(flags);
}
case 'eject': {
return ejectRoutes(flags);
}
default: {
console.error(`${errorLabel()} Unknown command ${chalk.red(cmd)}`);
return 1;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import fs from 'node:fs/promises';
import type { Options } from '../../../../astro/src/index.js';
import { parse, t, visit } from './babel.js';
import type { Options } from '../../../astro/src/index.js';
import { parse, t, visit, generate } from './babel.js';

export async function parseAstroConfig(astroConfigPath: string): Promise<t.File> {
const source = await fs.readFile(astroConfigPath, { encoding: 'utf-8' });
Expand All @@ -17,6 +17,17 @@ export async function parseAstroConfig(astroConfigPath: string): Promise<t.File>
return result;
}

export function generateAstroConfig(astroConfig: t.File): string {
const defaultExport = 'export default defineConfig';

let output = generate(astroConfig);

// add a new line
output = output.replace(defaultExport, `\n${defaultExport}`);

return output;
}

/**
* This function modifies the arguments provided to the tutorialkit integration in the astro
* configuration.
Expand Down Expand Up @@ -156,9 +167,9 @@ function updateObject(properties: any, object: t.ObjectExpression | undefined):

object ??= t.objectExpression([]);

for (const property of properties) {
for (const property in properties) {
const propertyInObject = object.properties.find((prop) => {
return prop.type === 'ObjectProperty' && prop.key === property;
return prop.type === 'ObjectProperty' && prop.key.type === 'Identifier' && prop.key.name === property;
}) as t.ObjectProperty | undefined;

if (!propertyInObject) {
Expand Down Expand Up @@ -191,6 +202,10 @@ function fromValue(value: any): t.Expression {
return t.numericLiteral(value);
}

if (typeof value === 'boolean') {
return t.booleanLiteral(value);
}

if (Array.isArray(value)) {
return t.arrayExpression(value.map(fromValue));
}
Expand Down
23 changes: 23 additions & 0 deletions packages/cli/src/utils/workspace-version.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
export function updateWorkspaceVersions(
dependencies: Record<string, string>,
version: string,
filterDependency: (dependency: string) => boolean = allowAll,
) {
for (const dependency in dependencies) {
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;
}
}
}
}

function allowAll() {
return true;
}
Loading