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

Pluggable CLI templates, add coreapp-cpp project template #10352

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

asklar
Copy link
Member

@asklar asklar commented Aug 1, 2022

Description

Today when creating a project from the CLI, we have a bit of a combinatorial matrix of possible options:

  • C# vs. C++
  • nuget vs. source
  • winui3 vs. system xaml
  • app vs. lib

It becomes hard to maintain to have to consider all 2^{N-1} combinations.
The solution I am proposing is that in addition to the options we have today (which we keep in the init CLI for back-compat), we add a new --template parameter. This is a string, that just gets passed down to the RNW CLI package, so each version of the RNW package can support a different set of templates.

This enables defining a template manifest template.json that can override the old options, as well as set any nuget packages, and provide its own set of project files and sources. This decouples us from the {shared, cpp, cs} x {lib, app} model, which we will also need for winappsdk (since the project files are very different from uwp's).

Type of Change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update
Microsoft Reviewers: Open in CodeFlow

@@ -83,14 +83,14 @@ export const windowsInitOptions = initOptions({
type: 'string',
describe: 'The type of project to initialize (supported on 0.64+)',
choices: ['app', 'lib'],
default: 'app',
default: undefined,
Copy link
Contributor

@jonthysell jonthysell Aug 1, 2022

Choose a reason for hiding this comment

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

I think for back-compat and test validation, it makes more sense here to leave this value as set, and update the description to include the range of versions, like supported on 0.64-0.70 or whatever.

type: 'string',
describe: '[Experimental] Specify a project template to use',
hidden: true,
default: undefined,
Copy link
Contributor

Choose a reason for hiding this comment

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

This has to be undefined because it has a conflicts field, but we should comment that here.

},
experimentalNuGetDependency: {
type: 'boolean',
describe:
'[Experimental] change to start consuming a NuGet containing a pre-built dll version of Microsoft.ReactNative',
hidden: true,
default: false,
default: undefined,
Copy link
Contributor

@jonthysell jonthysell Aug 1, 2022

Choose a reason for hiding this comment

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

IIRC, the args parsing code won't ever actually return undefined at runtime with a type set to 'boolean'. It will just return false. So setting this to undefined here is really misleading for devs, - which is why the tests fail - to stop devs from setting this to undefined.

Setting default to undefined only works for string options.

template: {
type: 'string',
describe: '[Experimental] Specify a project template to use',
hidden: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Not necessarily a problem with this PR, but I think having experimental options set with hidden: true makes it hard to get them correct since they don't appear in the help. Honestly I think the [Experimental] warning is enough to tell people to beware.

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.

None yet

2 participants