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

feat(crnl): store metadata with new projects #551

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

atlj
Copy link
Collaborator

@atlj atlj commented Apr 26, 2024

Summary

We need a way to track the project information since we are going to introduce a library upgrade helper. With this PR, when you create a new library using create-react-native-library, it will automatically save;

  1. Version of create-react-native-library used to create the project
  2. The library type (native module, native view, etc.)
  3. The languages used (Kotlin, objective-c, etc.)

into create-react-native-library.json file at the root.

Test plan

  1. Build and run create-react-native-library
  2. Note down the options you have picked
  3. Make sure the create-react-native-library.json file has been generated at the root of your library
  4. Check the version, library type, and languages fields and make sure they are using correct data.

@atlj atlj requested a review from satya164 April 26, 2024 09:50
tsconfig.json Outdated
@@ -1,4 +1,9 @@
{
"include": [
"packages/create-react-native-library/src",
"packages/create-react-native-library/package.json",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

import { version as crnlVersion } from '../package.json'; This line was generating errors during typecheck without including.


spinner.text = 'Writing metadata';
fs.writeJsonSync(
path.join(folder, 'create-react-native-library.json'),
Copy link
Member

Choose a reason for hiding this comment

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

Lets move to package.json

Comment on lines 798 to 800
crnlVersion,
languages,
projectType: type,
Copy link
Member

Choose a reason for hiding this comment

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

Let's include everything by default and then exclude some which are not needed

@@ -787,6 +794,21 @@ async function create(argv: yargs.Arguments<any>) {
}
}

const metadata: Metadata = {
crnlVersion,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
crnlVersion,
version,

Comment on lines +822 to +826
const createdPackageJson = fs.readJsonSync(path.join(folder, 'package.json'));
createdPackageJson['create-react-native-library'] = filteredAnswers;
fs.writeJsonSync(path.join(folder, 'package.json'), createdPackageJson, {
spaces: 2,
});
Copy link
Member

Choose a reason for hiding this comment

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

Let's use async fs methods.

const ignoredAnswers = ignoredArgs.map((argName) => questions[argName]?.name);
const standardArgs = ['_', '$0', 'name'];

const filteredAnswers = Object.fromEntries(
Copy link
Member

Choose a reason for hiding this comment

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

Maybe some name like libraryMetadata to make it more obvious what it is?

Suggested change
const filteredAnswers = Object.fromEntries(
const libraryMetadata = Object.fromEntries(

const standardArgs = ['_', '$0', 'name'];

const filteredAnswers = Object.fromEntries(
Object.entries(answers).filter(
Copy link
Member

Choose a reason for hiding this comment

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

You can do something like this to have better types and remove ts-expect-error

type AnswerEntries<T extends keyof Answers = keyof Answers> = [
  T,
  Answers[T],
][];

// ...

Object.entries(answers) as AnswerEntries).filter(

Object.entries(answers).filter(
([answer]) =>
!(
ignoredArgs.includes(answer as ArgName) ||
Copy link
Member

Choose a reason for hiding this comment

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

The casting here seems incorrect as answer will contain camelcase and here ignoredArgs are kebab-case. This should give a type error.

'react-native-version',
];
const ignoredAnswers = ignoredArgs.map((argName) => questions[argName]?.name);
const standardArgs = ['_', '$0', 'name'];
Copy link
Member

Choose a reason for hiding this comment

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

Maybe lets filter these out even before putting these in Answers so we only have the known keys.

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