refactor: resolvePlugins by combining resolveFarmPlugins and resolveAsyncPlugins #1274
Conversation
🦋 Changeset detectedLatest commit: 6e3d7a3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
ErKeLost
changed the title
ErKeLost
changed the title
packages/core/src/plugin/index.ts
Outdated
| let plugins = config.plugins ?? []; | ||
|
|
||
| // First, resolve any promises and flatten the array | ||
| plugins = await resolveAsyncPlugins(plugins); |
There was a problem hiding this comment.
We should first judge whether there is a plugins.
if (!plugins.length) {
return {
rustPlugins: [],
jsPlugins: []
};
}This code should be put in front.
packages/core/src/plugin/index.ts
Outdated
| @@ -9,8 +9,27 @@ import merge from '../utils/merge.js'; | |||
| export * from './js/index.js'; | |||
| export * from './rust/index.js'; | |||
|
|
|||
| export async function resolveFarmPlugins(config: UserConfig) { | |||
| const plugins = config.plugins ?? []; | |||
| export async function resolveAsyncPlugins<T>(arr: T[]): Promise<T[]> { | |||
There was a problem hiding this comment.
This method seems to just move the position, and we should put the important method at the top of the code
|
@ErKeLost Thanks for changing the PR name, I was getting a PR linter error. I don't understand why I was getting this error. Then I saw the github actions and realised I was missing the PR tags. Maybe that was causing the error. Does there exist PR docs like better PR for this project or an explanation for PR tagging something down this line? |
|
thanks |
Here are some pr title specifications, which I will add later in the contribution guide Available types:
- fix
- feat
- docs
- style
- refactor
- perf
- test
- build
- ci
- chore
- revert
- releaseThis ci, I'll just run again. then its will be fixed |
| @@ -0,0 +1,5 @@ | |||
| --- | |||
| '@farmfe/core': minor | |||
There was a problem hiding this comment.
We should only release the patch version of @ farmfe/ core. You need to rerun npx changeset.
|
@ErKeLost could you also tell me docs about the |
For all regular pr, we need to upgrade the patch version. Minor or major is generally not allowed. I will note it in CONTRIBUTING.md later. After typing npx changeset, select |
|
@ErKeLost I am resloving the cause of falling test case in the github actions. So, I will not updating the changeset to patch until it resolves. |
|
@DeepeshKalura hi, I updated the contribution guide. Thank you for your advice. I think what you said is very helpful, so I added a column in the document to make it more intuitive for users to see. If you think there's anything else to add, just let me know. |
|
@ErKeLost Hello, I see your updated contribution guide feels the same to me. I gave some advice damm! I even don't know why self 😅. If I do so, it will be great that my goal begins with helping this project. I was out for like 3 days hackathon in my college so I can't resolve this error until 11 May. If this issue is not resolved by that time then I will resolve that error and hope this PR merge Successfully. |
Description:
This PR introduces a new function, resolvePlugins, which combines the functionality of resolveFarmPlugins and resolveAsyncPlugins. This new function first resolves any promises and flattens the array of plugins, and then processes the resulting plugins as in resolveFarmPlugins. This allows the use of async and nested plugins in the configuration.
Changes:
Added a new function resolvePlugins in src/plugin/index.ts that handles async and nested plugins.
Replaced calls to resolveFarmPlugins with resolvePlugins in the codebase.
BREAKING CHANGE:
This change should not introduce any breaking changes. The new function resolvePlugins is backwards compatible with resolveFarmPlugins.
Related issue :
Resolves #1224