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

Move create-keystone-app package inside the monorepo #9102

Merged
merged 17 commits into from
Jul 18, 2024

Conversation

iamandrewluca
Copy link
Contributor

@iamandrewluca iamandrewluca commented Apr 17, 2024

PS: I was looking at create-payload-app maybe we can borrow that code for KS also, which will allow multiple KS templates

pnpm publish --dry-run
✔ You're on branch "feat/move-create-keystone-app" but your "publish-branch" is set to "master|main". Do you want to continue? (y/N) · true
npm notice
npm notice 📦  create-keystone-app@9.0.1
npm notice === Tarball Contents ===
npm notice 1.1kB LICENSE
npm notice 356B  README.md
npm notice 63B   cli.js
npm notice 295B  dist/create-keystone-app.cjs.d.ts
npm notice 6.6kB dist/create-keystone-app.cjs.dev.js
npm notice 200B  dist/create-keystone-app.cjs.js
npm notice 6.6kB dist/create-keystone-app.cjs.prod.js
npm notice 4.8kB dist/create-keystone-app.esm.js
npm notice 46B   dist/declarations/src/index.d.ts
npm notice 109B  dist/declarations/src/index.d.ts.map
npm notice 971B  package.json
npm notice 42B   starter/_gitignore
npm notice 2.7kB starter/auth.ts
npm notice 1.0kB starter/CHANGELOG.md
npm notice 1.0kB starter/keystone.ts
npm notice 397B  starter/package.json
npm notice 2.9kB starter/README.md
npm notice 9.5kB starter/schema.graphql
npm notice 1.1kB starter/schema.prisma
npm notice 5.2kB starter/schema.ts
npm notice 195B  starter/tsconfig.json
npm notice === Tarball Details ===
npm notice name:          create-keystone-app
npm notice version:       9.0.1
npm notice filename:      create-keystone-app-9.0.1.tgz
npm notice package size:  11.7 kB
npm notice unpacked size: 45.2 kB
npm notice shasum:        decfd7d53d50872d7d10daf98aaa27700786eb95
npm notice integrity:     sha512-CHG5zV1StXuDF[...]WvYe3RzlrnuEw==
npm notice total files:   21
npm notice
npm notice Publishing to https://registry.npmjs.org/ with tag latest and default access (dry-run)
+ create-keystone-app@9.0.1

@iamandrewluca iamandrewluca force-pushed the feat/move-create-keystone-app branch 3 times, most recently from 2ffc70a to c5f6d71 Compare April 17, 2024 20:51
Copy link

codesandbox-ci bot commented Apr 17, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit f4b7d78:

Sandbox Source
@keystone-6/sandbox Configuration

@iamandrewluca
Copy link
Contributor Author

@dcousens please take a look, and tell me please if I need to change something else.

@dcousens dcousens changed the title refactor: move create-keystone-app inside monorepo Move create-keystone-app package inside the monorepo Apr 24, 2024
@dcousens
Copy link
Member

dcousens commented Apr 24, 2024

I was looking at create-payload-app maybe we can borrow that code for KS also, which will allow multiple KS templates

I don't mind the idea of different templates, but let's focus on this being a 1:1 move into this monorepo from our create-keystone-app repository, wherever we can, and then we can talk about any UX improvements in a different pull request.

@dcousens
Copy link
Member

@iamandrewluca before I review this in depth, can you please highlight any changes you needed to make when porting this from https://github.com/keystonejs/create-keystone-app (as comments on the diff).

This will help me focus on what is different instead of reviewing the existing create-keystone-app code [which is needing a re-write, but that's a different discussion to have after this pull request]

Copy link
Contributor Author

@iamandrewluca iamandrewluca left a comment

Choose a reason for hiding this comment

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

The process of creating this package was to do these steps:

  • Make a copy of auth module
  • Cleanup everything from auth module and leave the bare minimum
  • Copy starter files from other repo
  • Copy source code from other repo
  • Apply the changes from this MR on source-code
  • Adjust package.json and cli.js to build and run ok with preconstruct

packages/create/src/index.ts Outdated Show resolved Hide resolved
packages/create/src/index.ts Outdated Show resolved Hide resolved
packages/create/package.json Show resolved Hide resolved
@iamandrewluca
Copy link
Contributor Author

iamandrewluca commented Apr 24, 2024

@dcousens I left some comments

PS: I synced the branch with the main from upstream. Please check if the pnpm lock file has been updated correctly (don't have much experience with pnpm)

On conflict I picked the version from main, then I run a pnpm install (usually this is what I do with npm on lock conflicts)

@dcousens dcousens force-pushed the feat/move-create-keystone-app branch from aa5a82f to 1659488 Compare July 18, 2024 05:10
@dcousens
Copy link
Member

Checking the diff against https://github.com/keystonejs/create-keystone-app, and removing a few deprecated files from create-keystone-app, and ignoring the package.json:

diff -r ../../../create-keystone-app/cli.js ./cli.js
2,3c2
< 'use strict';
< require('.');
---
> import './dist/create-keystone-app.esm.js'
diff -r ../../../create-keystone-app/src/index.ts ./src/index.ts
5c5
< import execa, { ExecaError } from 'execa';
---
> import execa from 'execa';
7,8d6
< import { checkVersion } from './checkVersion';
< import { UserError } from './utils';
10a9,11
> import { checkVersion } from './checkVersion';
> import { UserError } from './utils';
> import { fileURLToPath } from 'url';
11a13
> const __dirname = path.dirname(fileURLToPath(import.meta.url));
50c52,53
< const installDeps = async (cwd: string): Promise<'yarn' | 'npm'> => {
---
> async function installDeps (cwd: string) {
>   const pkgManager = (process.env.npm_config_user_agent ?? 'npm').split('/').shift()
52c55
<     'Installing dependencies with yarn. This may take a few minutes.'
---
>     `Installing dependencies with ${pkgManager}. This may take a few minutes.`
55,75c58,62
<     await execa('yarn', ['install'], { cwd });
<     spinner.succeed('Installed dependencies with yarn.');
<     return 'yarn';
<   } catch (_err: any) {
<     let err: ExecaError = _err;
<     if (err.failed) {
<       process.stdout.write('\n');
<       spinner.warn('Failed to install with yarn.');
<       spinner.start(
<         'Installing dependencies with npm. This may take a few minutes.'
<       );
<       try {
<         await execa('npm', ['install'], { cwd });
<         spinner.succeed('Installed dependencies with npm.');
<       } catch (npmErr) {
<         spinner.fail('Failed to install with npm.');
<         throw npmErr;
<       }
<       process.stdout.write('\n');
<       return 'npm';
<     }
---
>     await execa(pkgManager, ['install'], { cwd });
>     spinner.succeed(`Installed dependencies with ${pkgManager}.`);
>     return pkgManager;
>   } catch (err) {
>     spinner.fail(`Failed to install with ${pkgManager}.`);
78c65
< };
---
> }
116c103
<   - ${packageManager === 'yarn' ? 'yarn' : 'npm run'} dev
---
>   - ${packageManager} run dev

I'm happy with this!

@dcousens dcousens merged commit 0decc55 into keystonejs:main Jul 18, 2024
42 of 43 checks passed
@iamandrewluca
Copy link
Contributor Author

Hooray 🎉

@iamandrewluca iamandrewluca deleted the feat/move-create-keystone-app branch July 18, 2024 07:18
This was referenced Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants