-
Notifications
You must be signed in to change notification settings - Fork 29
feat: support for multiple entry points / multi-project support #199
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
Changes from all commits
9b5df0e
94b8c9b
4ddc7a5
ee42dc3
a4fe7b6
a327ce8
0c3ee0c
3a0f337
a43892f
25e75df
fbfbcf5
cad96ec
fc61f44
a9c3c21
bb68059
ec19c06
d04c34d
c8af621
058b273
49d2c47
138c1e0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,17 @@ | ||
| import { getEntryFromCliOrConfig } from './internal/get-entries-from-cli-or-config'; | ||
| import { getEntriesFromCliOrConfig } from './internal/get-entries-from-cli-or-config'; | ||
| import { cli } from './cli'; | ||
| import { getProjectData } from '../api/get-project-data'; | ||
| import getFs from '../fs/getFs'; | ||
|
|
||
| export function exportData(...args: string[]): void { | ||
| const fs = getFs(); | ||
| const entryFile = getEntryFromCliOrConfig(args[0], false); | ||
| const projectEntries = getEntriesFromCliOrConfig(args[0], true); | ||
|
|
||
| const data = getProjectData(entryFile, fs.cwd(), { | ||
| includeExternalLibraries: true, | ||
| }); | ||
| cli.log(JSON.stringify(data, null, ' ')); | ||
| for (const entry of projectEntries) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we need to adopt that one as well. How do we want to do it? There are some tools which use this function to load data. So we have to be careful. I see two options:
What is your opinion on that?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess option 1 will be the more "friendly" option as it will/should not break anything as you already pointed out. Option 2 we could keep in mind for a future major version and/or as alternative format for
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @rainerhahnekamp Nevertheless this would introduce a breaking change because How should be handle this?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure if
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Then I don't know what to do here. For introducing the new property
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, you are right. Sorry for the confusion. We need to have the possibility to pass on the projectName as well (if there are more projects involved).
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
| const data = getProjectData(entry.entry.fileInfo.path, fs.cwd(), { | ||
| includeExternalLibraries: true, | ||
| projectName: entry.projectName, | ||
| }); | ||
| cli.log(JSON.stringify(data, null, ' ')); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| export type Entry<TEntry> = { | ||
| projectName: string; | ||
| entry: TEntry; | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import { Configuration } from '../../config/configuration'; | ||
| import { isEmptyRecord } from '../../util/is-empty-record'; | ||
|
|
||
| export function parseEntryPointsFromCli( | ||
| entryFileOrEntryPoints: string, | ||
| sheriffConfig: Configuration, | ||
| ): Record<string, string> | undefined { | ||
| const entryPointsFromConfig = sheriffConfig.entryPoints; | ||
| if (entryFileOrEntryPoints.includes(',')) { | ||
| if (!entryPointsFromConfig || isEmptyRecord(entryPointsFromConfig)) { | ||
| return undefined; | ||
| } | ||
| const splittedEntries = entryFileOrEntryPoints.split(','); | ||
| const entryPoints: Record<string, string> = {}; | ||
|
|
||
| for (const entry of splittedEntries) { | ||
| const trimmedEntry = entry.trim(); | ||
| const entryPoint = entryPointsFromConfig[trimmedEntry]; | ||
| if (entryPoint) { | ||
| entryPoints[trimmedEntry] = entryPoint; | ||
| } | ||
| } | ||
| return entryPoints; | ||
| } | ||
|
|
||
| // If no comma is found, it could be a single entry point | ||
| if ( | ||
| entryPointsFromConfig && | ||
| entryFileOrEntryPoints in entryPointsFromConfig | ||
| ) { | ||
| const singleEntryPoint = entryPointsFromConfig[entryFileOrEntryPoints]; | ||
| return { [entryFileOrEntryPoints]: singleEntryPoint }; | ||
| } | ||
|
|
||
| return undefined; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While we only had
entryFile, it felt acceptable to briefly mention it in a sentence at the end.Now, with the introduction of
entryPoints, the situation has become a bit more complex. I think it would be better to dedicate a separate chapter at the end of the page that explains bothentryFileandentryPoints. This would allow us to elaborate on their usage and provide examples.We could then also keep the
verify,list, andexportsections more concise by simply referring to this new chapter.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done