feat: package manager support create amplify #793

0618 · 2023-12-07T22:12:16Z

Issue #, if available:

Description of changes:

With this PR, user should be able to use npm, yarn (v1, v2-4), pnpm to create amplify project by running:

npm create amplify
yarn create amplify
pnpm create amplify

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

changeset-bot · 2023-12-07T22:12:20Z

⚠️ No Changeset found

Latest commit: a8e38af

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

0618 · 2023-12-07T22:14:00Z

packages/create-amplify/src/create_amplify.ts

+    return packageManagers['npm'];
+  }
+
+  const userAgent = process.env.npm_config_user_agent;


We only use npm_config_user_agent for now in this PR. I'll make a follow-up PR to also detect the lock file.

Is there some way we can detect which package manager the customer ran the create amplify command with? There won't necessarily be a lock file at this point.

npm_config_user_agent is the one user use to run create amplify.

I'd like to also use the lock file just in case they are different.

packages/create-amplify/src/package_manager.ts

packages/create-amplify/src/package_manager_controller.ts

packages/create-amplify/src/project_initializer.ts

packages/create-amplify/src/package_manager_controller.ts

packages/create-amplify/src/package_manager.ts

...ges/create-amplify/src/package-manager-controller/yarn_classic_package_manager_controller.ts

packages/create-amplify/src/amplify_project_creator.ts

packages/create-amplify/src/execute_with_logger.ts

packages/create-amplify/src/package_manager.ts

packages/create-amplify/src/create_amplify.ts

edwardfoyle

This is getting really, really close! Just one more change to simplify the PackageManagerController and remove the need for the PackageManagerControllerType. Also a few more testing suggestions.

packages/create-amplify/src/create_amplify.ts

packages/create-amplify/src/initial_project_file_generator.ts

...ges/create-amplify/src/package-manager-controller/package_manager_controller_factory.test.ts

packages/create-amplify/src/package-manager-controller/package_manager_controller_factory.ts

packages/create-amplify/src/package-manager-controller/package_manager_controller.ts

...ges/create-amplify/src/package-manager-controller/package_manager_controller_factory.test.ts

edwardfoyle · 2023-12-21T19:46:02Z

packages/create-amplify/src/package-manager-controller/npm_package_manager_controller.ts

+/**
+ * NpmPackageManagerController is an abstraction around npm commands that are needed to initialize a project and install dependencies
+ */
+export class NpmPackageManagerController extends PackageManagerController {


we should have a test file for each of these implementors that asserts expected behavior of the methods. Even though they are all basically the same, these checks are important to ensure that each subclasses continues to work as expected as the base class evolves.

We could also consider a little test matrix similar to what I outlined for the controller factory. Although this one would be quite a bit more complicated so maybe better to just write tests for each subclass individually.

edwardfoyle · 2023-12-22T19:57:25Z

...ges/create-amplify/src/package-manager-controller/package_manager_controller_factory.test.ts

+      {
+        name: 'pnpm',
+        userAgent: 'pnpm/5.0.0 node/v15.0.0 darwin x64',
+        expectedInstanceof: PackageManagerController,


Suggested change

expectedInstanceof: PackageManagerController,

expectedInstanceof: PnpmPackageManagerController,

Same applies to the yarn classic and yarn modern test case as well

Good catch!

...ges/create-amplify/src/package-manager-controller/yarn_classic_package_manager_controller.ts

...ages/create-amplify/src/package-manager-controller/yarn_modern_package_manager_controller.ts

sobolk

LGTM.
Please address remaining comment.

sobolk · 2023-12-26T18:57:25Z

...ages/create-amplify/src/package-manager-controller/yarn_modern_package_manager_controller.ts

+    fs.writeFile(this.path.resolve(targetDir, 'yarn.lock'), '', (err) => {
+      if (err) {
+        logger.error(`Error creating ${targetDir}/yarn.lock ${err.message}}`);
+      } else {
+        logger.info(`${targetDir}/yarn.lock created successfully.`);
+      }
+    });


I'm not sure why linter didn't catch this but we shouldn't use fs apis that use callbacks.
We await addLockFile above but we don't await anything inside - this creates potential race conditions.
The fs.writeFile (from fs) does not return Promise and can't be awaited - we should use fs/promises instead.
When we convert this to fsp we should use try catch here.

As for linter - mind creating GH issue to add (or fix?) linter to enforce file system api usage?

Done a146fa1

feature request: #847

sobolk · 2023-12-27T17:39:04Z

packages/create-amplify/src/package-manager-controller/package_manager_controller.ts

+    protected executable: string,
+    protected binaryRunner: string,
+    protected initDefault: string[],
+    protected installCommand: string,


these can be read only (here and there if this repeats)

Oops...just merged. I'll address it in the next PR

Followed up here #901 (comment)

* feat: create amplify skip prompts for `--yes` option (#507) * feat: add env ci support * test: update unit tests * feat: create amplify --yes option * chore: remove un-used code * feat: support Panage Manager env var * chore: add comments * chore: remove env.CI * chore: update changeset * chore: rename PACKAGE_MANAGER_EXECUTABLE * fix: tests * fix npm -> npx * feat: init e2e test workflow for Package Manager Support (#533) * test: init e2e flow test * fix GH hash * use dynamic pkg manager * setup nodejs with npm * setup more pkg managers * install pkg managers * fix yarn1 and change step name * fix yarn1 * fix yarn1 * split yarn1 into 2 steps * fix yarn1 * fix yarn1 win * exclude bun on windows * yarn1 windows * setup yarn3 * yarn3 * yarn3 yarnPath * yarn use 3rd party action * try yarn3 * set yarn 3.6.x * set yarn berry * set yarn stable and pass --yes * feat: add env ci support * test: update unit tests * feat: create amplify --yes option * chore: remove un-used code * feat: support Panage Manager env var * chore: add comments * chore: remove env.CI * chore: update changeset * chore: rename PACKAGE_MANAGER_EXECUTABLE * fix: tests * use Env Var PACKAGE_MANAGER_EXECUTABLE * fix npm -> npx, remove bun * fix npm -> npx * change -- --yes * chore: update package.lock * chore: remove @Alpha * fix: clean up yarn * fix: change yarn1 to yarn * chore: rename yarn step * chore: use env * fix: yarn-stable env var * add comments * chore: update sample app with new imports (#541) * feat: use "use client"; directive in generated react components (#540) * fix: flatten prop types auth (#534) * fix: flatten auth login with types * chore: update snapshots * chore: update api * chore: add changeset * chore: add TODO --------- Co-authored-by: Amplifiyer <[email protected]> Co-authored-by: Spencer Stolworthy <[email protected]> Co-authored-by: awsluja <[email protected]> * fix: pnpm init and env var (#563) * fix: npm_project_initializer to use env var * fix: pnpm init * chore: update package.lock * chore: update package.lock * test: use GH workflow for e2e test of create amplify (#636) * feat: setup GH workflow for create amplify * chore: changeset * fix: add pkg type for modern yarn * fix npm install * feat: test Node v20 and Node v19 * feat: test Node v20 and yarn-stable with Node 19 * feat: exclude yarn-stable * feat: include node 20 and exclude node 19 * feat: use node 20 for others, use node 19 for yarn-stable * chore: add TODO to use Node 20 * fix: e2e tests (#771) * chore: change branch to poc/pms-create-amplify * fix: gitIgnore test * fix: refactor e2e for pms * chore: update package.lock * chore: update package.lock * temp: refactor * temp: run 1 test * temp: fix npx * temp: install packages for yarn * temp: update initial_project_file_generator * temp: install ts for yarn * temp: ignore node_modules and yarn.lock * temp: create yarn.lock for yarn stable * temp: fix yarn-stable init * temp: fix --help * fix: assert for gitignore * chore: change yarn stable install * fix: not install yarn stable globally * add @yarnpkg/sdks base * yarn stable use node-modules * remove emoji * remove @yarnpkg/sdks * yarn stable use node 18.18 * yarn stable use node 20 * chore: re-enable all tests * Revert "chore: re-enable all tests" This reverts commit f7a167b. * chore: uncomment test * chore: enable all initialStatues * test: change concurrencyLevel for yarn and yarn stable * add yarn-stable to test fails fast * chore: update changeset * chore: change workflow trigger * chore: yarn not install typescript in root folder * chore: update package.lock * Revert "chore: yarn not install typescript in root folder" This reverts commit 0d83dd0. * chore: remove comment * chore: fix typo * chore: rename yarn-classic and yarn modern * fix(create_amplify.test): after merge main * fix the concurrency for yarn classic and modern, and fix expect * feat: package manager support create amplify (#793) * chore: update package.lock * init * feat: dynamically get package manager * chore: change workflow trigger * chore: change workflow trigger * chore: rename npm functions * chore: update output log * fix: pnpm cache clear command * chore: refactor packageManager * fix: remove pnpm store * chore: create package_manager file * chore: change workflow trigger * chore: refactor PackageManager Controller and Initializer * fix: update types * chore: refactor to use factory * chore: remove try catch from the controllers * chore: change type * chore: use abstract class * fix tests * fix: PackageManagerBase * chore: refactor PackageManagerControllerFactory * chore: refactor packageJsonExists * chore: cleanup code * fix: projectJsonExists * remove package_manager file * chore: refactor package_manager props into package manager controllers * chore: rename packageManagerControllerFactory * chore: make projectRoot, userAgent, getPackageManagerName private * chore: refactor ensureInitialized * chore: refactor PackageManagerController to extend PackageManagerControllerFactory * Revert "chore: refactor PackageManagerController to extend PackageManagerControllerFactory" This reverts commit 7ddc64f. * chore: refactor to inject PackageManagerControllerFactory to xPackageManagerController * chore: move initialProjectFileGenerator to packageManagerController * fix: initializeAMplifyFolder * fix: generateInitialProjectFiles * fix: template path * chore: refactor getPackageManagerController yarn-modern * chore: create getWelcomeMessage * Revert "chore: refactor getPackageManagerController yarn-modern" This reverts commit 28da8a9. * chore: refactor generateInitialProjectFiles for yarn-modern * chore: cleanup code * chore: remove PackageManagerControllerFactory index * chore: remove ensureInitialized * Revert "chore: remove ensureInitialized" This reverts commit 96483ce. * Revert "Revert "chore: remove ensureInitialized"" This reverts commit 7c59c80. * temp: add initializeProject to PackageManagerController * temp: revert InitialProjectFileGenerator * fix: InitialProjectFileGenerator * test: restore initial_project_file_generator * move installDependencies and getWelcomeMessage to PackageManagerController * add addLockFile * add JSDocs and resolve some review comments * fix: create_amplify.test * chore: add addTypescript * chore: refactor generateInitialProjectFiles * Update packages/create-amplify/src/package-manager-controller/package_manager_controller_factory.ts Co-authored-by: Edward Foyle <[email protected]> * chore: refactor generateInitialProjectFiles again * chore: refactor welcomeMessage * test: add package_manager_controller_factory * test: fix test types * chore: comments update * chore: handle process.env.npm_config_user_agent undefined * chore: move addLockFile and addTypescript into initializeTsConfig * test: refactor packageManagerControllerFactory * fix: yarn initializeTsConfig * fix: yarn modern initializeTsConfig * fix: amplify_project_creator test * chore: refactor contructor * chore: update package.lock * fix: pnpm init, remove --debug, etc * test: add test for NpmPackageManagerController * fix build error * chore: refactor NpmPackageManagerController * test: add test for xPackageManagerController * chore: convert fs to fsp * fix yarn modern --------- Co-authored-by: Edward Foyle <[email protected]> * chore: update package-lock * test: add deploy test (#901) * chore: add e2e_flow.test * add deploy test but don't work * comment out deploy test * fix: e2e test assert * chore: update package.lock * add deploy test * use before and after * change amplifyCli to execa * change test trigger * Revert "change amplifyCli to execa" This reverts commit 9f85439. * setupProfile * Revert "Revert "change amplifyCli to execa"" This reverts commit 4950b7d. * remove fail test and initialStates * install @aws-amplify/backend-deployer * chore: update changeset * chore: set nodeLinker in test project * fix execaOptions syntax error * fix: npx in cdk deployer * chore: update changeset * fix: yarn modern build * chore: update package-lock * chore: remove some asserts * chore: remove synth * install packages to fix yarn * remove create-amplify help * simplify before and after * remove one packageManagerSetup * refactor packageManagerSetup * fix yarn- * move yarn add to setupPackageManager * fix type error * Revert "fix type error" and "move yarn add to setupPackageManager" This reverts commit d88ef61. * try fix pnpm for windows by adding @AWS-SDK+credential-providers * remove the pnpm patch * add nodir to dictionary * move setupPackageManager to a new file * rename PACKAGE_MANAGER_EXECUTABLE to PACKAGE_MANAGER * refactor packageManagerSetup to use switch * refactor packageManagerSetup to initializeX * use beforeEach and afterEach * add TODO comment * temp * simplify yarn-modern treatments * misc changes * add TODO comments * exclude pnpm on windows * Revert "exclude pnpm on windows" This reverts commit dd7b423. * exclude pnpm on windows * resolve review comments * fix syntax error * address review comments * move this.packageManager logic to ctor * fix yarn-modern * add packageManagerCli and packageManagerExecutable * fix yarn * address review comments * rename package_manager_sanity_check * update workflow yml * chore: update package-lock * move npm proxy to before and after * change comments * feat(backend-deploy): use packageManagerController (#947) * fix logger * fix logLevel * change workflow trigger to include poc/pms-cli-core * move package_manager_controller to cli-core * chore: update changeset * move npm_config_user_agent to getPackageManagerName and add runWithPackageManager * fix build error * modify executeWithDebugLogger * add getPackageManagerCommandArgs * refactor cdk_deployer to use package manager * install cli-core * fix execaChildProcess type * use PackageManagerControllerFactory in cdkDeployer * fix amplify/amplify path * chore: update API.md * clean up package-manager-controller export * update API.md * add type PackageManagerController * use PackageManagerController type from plugin-types * rename PackageManagerControllerBase and use plugin-types * change workflow trigger to exclude poc/pms-cli-core * refactor BackendDeployerFactory to take PackageManagerController * clean up getPackageManagerCommandArgs * add projectRoot and rename packageManagerControllerBase file * set projectRoot default value * rename execute_with_debugger_logger * use execa Options * Revert "use execa Options" This reverts commit 30cd61e. * rename executeChildProcessWithPackageManager to executeCommand * make BackendDeployerFactory param mandatory * make BackendDeployerFactory param mandatory * remove DependencyType * remove @aws-amplify/platform-core from cli-core * remove some default projectRoot * remove all default projectRoot * inject printer to factory * update package-lock * fix cli-core version in backend-deployer * move @aws-amplify/cli-core to devDep * add getCommand * change execa option type * change projectRoot to private * rename projectRoot to cwd * change ./ to cwd * remove @aws-amplify/cli-core from backend-deployer * patch dependencies_validator for execa * fix getCommand, cdk_deployer.test, amplify_project_creator.test * fix initial_project_file_generator.test * fix executeWithDebugLogger test * fix YarnModernPackageManagerController test * fix packageManagerControllerFactory test * fix lint error * Update packages/cli-core/src/package-manager-controller/package_manager_controller_factory.ts Co-authored-by: Kamil Sobol <[email protected]> * remove never used packageManagerExecutable * update changeset * feat: add Package Manager test to health_checks (#968) * add package-manager-tests to health-checks * remove poc-e2e-flow-test * add run_package_manager_tests_tests condition * fix run_package_manager_tests_tests condition * remove amplify-backend_windows-latest_8-core * use re-usable action for run-e2e * Revert "remove amplify-backend_windows-latest_8-core" This reverts commit 547834a. * Revert "use re-usable action for run-e2e" This reverts commit 3159e01. * revert re-usable action * Update .changeset/new-timers-warn.md Co-authored-by: Kamil Sobol <[email protected]> * clean up commented code * use amplify-backend_windows-latest_8-core * Update .changeset/new-timers-warn.md Co-authored-by: Edward Foyle <[email protected]> * remove inaccurate JSdoc * remove integration-tests from changeset --------- Co-authored-by: Amplifiyer <[email protected]> Co-authored-by: Spencer Stolworthy <[email protected]> Co-authored-by: awsluja <[email protected]> Co-authored-by: Edward Foyle <[email protected]> Co-authored-by: Kamil Sobol <[email protected]>

0618 added 12 commits December 6, 2023 15:11

chore: update package.lock

ca8fa66

init

3b737a5

feat: dynamically get package manager

fdc9016

chore: change workflow trigger

dfeb8a9

chore: change workflow trigger

614abb8

chore: rename npm functions

54647a1

chore: update output log

5ebbbc2

fix: pnpm cache clear command

df057c8

chore: refactor packageManager

61e5f23

fix: remove pnpm store

ffa1e83

chore: create package_manager file

e6c099c

chore: change workflow trigger

a3a54c3

0618 requested review from edwardfoyle, sobolk, Amplifiyer and srquinn21 as code owners December 7, 2023 22:12

0618 commented Dec 7, 2023

View reviewed changes

edwardfoyle reviewed Dec 7, 2023

View reviewed changes

0618 changed the title ~~Poc/pms create amplify~~ feat: package manager support create amplify Dec 7, 2023

sobolk reviewed Dec 8, 2023

View reviewed changes

packages/create-amplify/src/package_manager_controller.ts Outdated Show resolved Hide resolved

0618 added 2 commits December 11, 2023 13:53

chore: refactor PackageManager Controller and Initializer

40f3590

fix: update types

b491902

edwardfoyle reviewed Dec 12, 2023

View reviewed changes

packages/create-amplify/src/package_manager.ts Outdated Show resolved Hide resolved

...ges/create-amplify/src/package-manager-controller/yarn_classic_package_manager_controller.ts Outdated Show resolved Hide resolved

sobolk reviewed Dec 12, 2023

View reviewed changes

0618 added 5 commits December 11, 2023 20:31

chore: refactor to use factory

9e3837c

chore: remove try catch from the controllers

b5665ec

chore: change type

3a50376

chore: use abstract class

7ec208b

fix tests

5fdea3c

0618 added 5 commits December 20, 2023 14:24

chore: refactor generateInitialProjectFiles again

29c0505

chore: refactor welcomeMessage

3667fd2

test: add package_manager_controller_factory

cb32ec7

test: fix test types

0acb711

chore: comments update

7d6a6d6

edwardfoyle reviewed Dec 21, 2023

View reviewed changes

0618 added 3 commits December 22, 2023 09:11

chore: handle process.env.npm_config_user_agent undefined

a6b0cf1

chore: move addLockFile and addTypescript into initializeTsConfig

def594d

test: refactor packageManagerControllerFactory

e98dbb2

edwardfoyle reviewed Dec 22, 2023

View reviewed changes

0618 added 3 commits December 25, 2023 20:32

fix: yarn initializeTsConfig

9e3a8b6

fix: yarn modern initializeTsConfig

33a5961

fix: amplify_project_creator test

46c2f89

sobolk reviewed Dec 26, 2023

View reviewed changes

0618 added 10 commits December 26, 2023 16:48

chore: refactor contructor

73b881a

Merge branch 'poc/package-manager-support' into poc/pms-create-amplify

08a09a2

chore: update package.lock

4bca631

fix: pnpm init, remove --debug, etc

489e84f

test: add test for NpmPackageManagerController

a83349d

fix build error

6ce0e18

chore: refactor NpmPackageManagerController

0b3eb4d

test: add test for xPackageManagerController

eeb8622

chore: convert fs to fsp

a146fa1

fix yarn modern

a8e38af

sobolk approved these changes Dec 27, 2023

View reviewed changes

0618 merged commit 548f9d1 into aws-amplify:poc/package-manager-support Dec 27, 2023
16 checks passed

0618 mentioned this pull request Jan 5, 2024

test: Package Manager Support for deploy #869

Closed

4 tasks

0618 mentioned this pull request Jan 17, 2024

test: add deploy test #901

Merged

5 tasks

0618 mentioned this pull request Jan 30, 2024

feat: package manager support #960

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: package manager support create amplify #793

feat: package manager support create amplify #793

0618 commented Dec 7, 2023

changeset-bot bot commented Dec 7, 2023 •

edited

Loading

0618 Dec 7, 2023

edwardfoyle Dec 7, 2023

0618 Dec 8, 2023

edwardfoyle left a comment

edwardfoyle Dec 21, 2023 •

edited

Loading

0618 Dec 27, 2023

edwardfoyle Dec 22, 2023

0618 Dec 22, 2023

sobolk left a comment

sobolk Dec 26, 2023

0618 Dec 27, 2023

0618 Dec 27, 2023

sobolk Dec 27, 2023

0618 Dec 27, 2023

0618 Jan 17, 2024

	expectedInstanceof: PackageManagerController,
	expectedInstanceof: PnpmPackageManagerController,

feat: package manager support create amplify #793

feat: package manager support create amplify #793

Conversation

0618 commented Dec 7, 2023

changeset-bot bot commented Dec 7, 2023 • edited Loading

⚠️ No Changeset found

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edwardfoyle left a comment

Choose a reason for hiding this comment

edwardfoyle Dec 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sobolk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

changeset-bot bot commented Dec 7, 2023 •

edited

Loading

edwardfoyle Dec 21, 2023 •

edited

Loading