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.

Sign up for GitHub

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

Jump to bottom

feat(cli): ENG-5881 dynamic cli options #2773

Merged
merged 3 commits into from
Dec 19, 2024
Merged

feat(cli): ENG-5881 dynamic cli options #2773

merged 3 commits into from
Dec 19, 2024

Conversation

crherman7
Copy link
Contributor

@crherman7 crherman7 commented Dec 4, 2024 •
edited
Loading

Describe your changes

This PR enhances Flagship Code by introducing dynamic environment configuration and validation, along with improvements to iOS and Android plugins. The changes streamline the handling of environment-specific builds and improve the flexibility of CLI commands.

Key Changes

  1. Dynamic Environment Configuration:

    • Added the ability to define environments dynamically through the .flagshipappenvrc file.
    • New singleEnv flag to enforce single environment selection during release builds.
    • Plugins now validate required options and write environment configurations automatically.

  2. Plugin Enhancements:

    • Added custom error handling (MissingOptionError) for missing plugin options.
    • Introduced validateOptions and writeEnvConfig helper functions for managing environment configurations in iOS and Android plugins.

  3. New CLI Command Options:

    • Added support for --app-env-initial, --app-env-dir, and --app-env-hide flags to customize environment settings dynamically.
    • Schema validation for flagship-code.commands.json to ensure CLI options conform to a standard structure.

  4. Dependency Checks:

    • Plugins and transformers now conditionally apply changes based on the presence of @brandingbrand/fsapp dependency.
    • Added helper functions hasDependency and matchesFilePatterns for dependency-based filtering.

  5. Testing and Configuration:

    • Updated Jest environment options in test files.
    • Added new fixtures for project-pbxproj tests.
    • Updated dependencies (find-node-modules, glob, ajv) for improved file and schema handling.

Issue ticket number and link

Type of change

  • New feature (non-breaking change which adds functionality)

Test Plan

  • run prebuild and run apps to verify correct environments

Checklist before requesting a review

  • A self-review of my code has been completed
  • Tests have been added / updated if required

@crherman7 crherman7 marked this pull request as draft December 4, 2024 22:22
@crherman7 crherman7 changed the title feat(cli): dynamic cli options feat(cli): ENG-5881 dynamic cli options Dec 4, 2024
@crherman7 crherman7 changed the base branch from develop to feature/app-router December 6, 2024 18:55
@crherman7 crherman7 changed the base branch from feature/app-router to develop December 6, 2024 18:55
@crherman7 crherman7 changed the base branch from develop to feature/app-router December 6, 2024 18:56
@crherman7 crherman7 force-pushed the feat/ENG-5881 branch 5 times, most recently from 314b661 to c191bce Compare December 11, 2024 22:37
@crherman7 crherman7 marked this pull request as ready for review December 16, 2024 20:06
@crherman7 crherman7 force-pushed the feat/ENG-5881 branch 2 times, most recently from 4717ca8 to 462b75f Compare December 17, 2024 16:59
crherman7
crherman7 commented Dec 17, 2024
View reviewed changes
packages/app-env/.flagshipcoderc.json
"example": "--app-env-initial development"
},
{
"flags": "--app-env-dir <value>",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We technically could aggregate this with babel traversal and looking at imports etc.

crherman7
crherman7 commented Dec 17, 2024
View reviewed changes
packages/cli/src/lib/commands.ts Outdated
/**
* The name of a global function to parse the input value.
*/
parse?: string;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

remove - decided not to use

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

NickBurkhartBB
NickBurkhartBB reviewed Dec 18, 2024
View reviewed changes
packages/app-env/.flagshipcoderc.json Show resolved Hide resolved
packages/cli/src/lib/commands.ts Outdated
const aggregatedOptions: CommandOption[] = [];

if (configFiles.length === 0) {
console.warn(`No ${CONFIG_FILE_NAME} files found.`);
Copy link
Contributor

Choose a reason for hiding this comment

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

does this need to be warning? can't we just have plugins without custom options?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah true - I will remove this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

packages/cli/src/transformers/transformers-0.73/ios/project-pbxproj.ts Outdated
@@ -8,7 +8,8 @@ import {
} from '@brandingbrand/code-cli-kit';
import type {XcodeProject, PBXFile} from 'xcode';

import {Transforms, defineTransformer} from '@/lib';
import {FSAPP_DEPENDENCY, Transforms, defineTransformer} from '@/lib';
import {hasDependency, matchesFilePatterns} from '@/lib/dependencies';
Copy link
Contributor

Choose a reason for hiding this comment

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

matchesFilePatterns unused

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's used in the fileFilter

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not seeing fileFilter in this file either

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ahh was looking at wrong file - good catch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

apps/example/coderc/plugins/plugin-env/src/index.ts
}
if (!options.appEnvDir) {
throw new MissingOptionError('appEnvDir');
}
Copy link
Contributor

Choose a reason for hiding this comment

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

we probably also need to verify that these folders exist and the initial environment exists in the app environment directory. currently it seems like it works fine if I input invalid values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah we could add additional validation of the directory

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@NickBurkhartBB NickBurkhartBB merged commit 76cda37 into brandingbrand:feature/app-router Dec 19, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NickBurkhartBB NickBurkhartBB NickBurkhartBB approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@crherman7 @NickBurkhartBB