feat: mfsdk #699
feat: mfsdk #699
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces significant changes to the Azusa Player Mobile project, focusing on restructuring the MusicFree plugin system. The modifications include removing existing MusicFree-related modules, introducing new utility functions for SDK management, updating type definitions, and refactoring components to handle dynamic music source plugins. The changes aim to provide a more flexible and modular approach to integrating music search and playback functionality from various sources. Changes
Sequence DiagramsequenceDiagram
participant User
participant SearchBar
participant MFsdkStore
participant EvalSDK
participant MediaFetcher
User->>SearchBar: Initiate Search
SearchBar->>MFsdkStore: Retrieve Available SDKs
MFsdkStore-->>SearchBar: Return SDK List
SearchBar->>EvalSDK: Load Plugin
EvalSDK->>MediaFetcher: Fetch Media
MediaFetcher-->>SearchBar: Return Search Results
SearchBar->>User: Display Results
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🔭 Outside diff range comments (1)
src/types/request.d.ts (1)
Line range hint
11-17: Remove duplicate interface declarationThe
UpdateLyricMappinginterface is declared twice in theNoxLyricnamespace. Remove the duplicate declaration to avoid potential confusion.- interface UpdateLyricMapping { - resolvedLrc?: NoxLyric.NoxFetchedLyric; - newLrcDetail?: Partial<NoxMedia.LyricDetail>; - lrc: string; - song: NoxMedia.Song; - currentTimeOffset: number; - }Also applies to: 46-52
♻️ Duplicate comments (1)
__tests__/musicfree/mfexample.js (1)
97-97:⚠️ Potential issueHard-coded credentials detected.
This repeats a previously noted security concern about the exposed API key. Consider moving the key to a secure location (e.g., environment variable).🧰 Tools
🪛 Gitleaks (8.21.2)
97-97: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 GitHub Check: CodeQL
[failure] 97-97: Hard-coded credentials
The hard-coded value "f3ac5b086f3eab260520d8e3049561e6" is used as key.
🧹 Nitpick comments (27)
src/utils/mfsdk.ts (3)
8-9: Use a more self-documenting subfolder constant name, if possible.
Though not a pressing matter, consider renaming "mfsdkSubFolder" to something that clarifies its purpose, for example, "MFSDK_SUBDIRECTORY".
21-24: Consider concurrency or chunked addition of SDK paths.
If the number of paths grows large, saving them all at once might cause performance issues in some storage engines. Think about splitting large arrays or handling concurrency if needed.
26-40: Validate partial load errors during initialization.
When partially successful in loading some SDKs, you return only the non-undefined ones. For clarity, consider logging which ones failed to load to aid debugging.src/utils/mediafetch/evalsdk.ts (2)
12-36: Document the difference between missing or partial implementations.
The commented-out methods (lines 24–35) hint that these functionalities are not needed for your app, but consider adding short doc comments to clarify that they are intentionally not used.
73-91: Handle failing URL resolution gracefully.
Currently, if all qualities fail, you throw an error. Consider returning a fallback or letting the calling function handle partial fallbacks (like trying direct links or skipping the track).src/components/setting/appearances/SkinSettings.tsx (2)
126-127: Avoid duplicating getThemeID.
You define getThemeID in multiple places (SkinItem and again here). Consider extracting it into a shared utility to reduce duplication.- const getThemeID = (skin: NoxTheme.Style) => - `${skin.metaData.themeName}.${skin.metaData.themeAuthor}`; + // Example in a shared utility file, e.g. useThemeUtils.ts + export function getThemeID(skin: NoxTheme.Style): string { + return `${skin.metaData.themeName}.${skin.metaData.themeAuthor}`; + }
170-170: Review the search bar’s responsibilities.
Check for duplication with other search bars or if additional props might unify the approach across the app.src/components/setting/developer/DeveloperSettings.tsx (1)
82-82: Consolidate or rename the Home component.
“Home” is fairly generic. If this is specifically “DeveloperHome,” renaming might improve clarity.__tests__/musicfree/mfexample.js (5)
1-1: Remove redundant 'use strict' directive.
Modern JavaScript modules automatically run in strict mode.- 'use strict';🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
51-51: Use Array.isArray(e) instead of e instanceof Array.
This is generally more reliable across different contexts.- e instanceof Array + Array.isArray(e)
63-63: Avoid direct usage of hasOwnProperty from target object.
Prefer using Object.hasOwn() or a safer approach.- if (e.hasOwnProperty(n)) { + if (Object.hasOwn(e, n)) {🧰 Tools
🪛 Biome (1.9.4)
[error] 63-63: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
🪛 eslint
[error] 63-63: Do not access Object.prototype method 'hasOwnProperty' from target object.
(no-prototype-builtins)
86-86: Avoid assignments in expressions to improve clarity.
Separate the assignment into its own statement.- this._parameters[e] || (this._parameters[e] = []), - this._parameters[e].push(n); + if (!this._parameters[e]) { + this._parameters[e] = []; + } + this._parameters[e].push(n);🧰 Tools
🪛 Biome (1.9.4)
[error] 86-86: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
🪛 eslint
[error] 86-87: Expected an assignment or function call and instead saw an expression.
(@typescript-eslint/no-unused-expressions)
523-533: Review unhandled branch in switch-like construct.
Here we only handle 'music', 'album', 'sheet', 'artist'. If new types are introduced later, consider how they should be handled by this function.src/hooks/useNavigation.ts (1)
25-28: Consider consolidating navigation methodsHaving two navigation methods with similar functionality increases maintenance overhead. Consider making the icon setting optional in the main
navigatemethod instead.- const navigate2 = (route: unknown) => - navigationGlobal.navigate(route as never); - - return { navigate, navigate2, getState: navigationGlobal.getState }; + return { navigate, getState: navigationGlobal.getState };Then update the
navigatemethod to makesetIcontruly optional:const navigate = ({ route, setIcon = true, options }: NavigateProps) => { // @ts-expect-error nav v7 hasnt fixed this type yet navigationGlobal.navigate(route as never, options); - if (setIcon) { + if (setIcon && IconMap[route]) { setRoute(IconMap[route]); } };src/components/setting/developer/plugins/musicfree/MusicFreeButton.tsx (1)
27-34: Consider using theme-based stylingHardcoded dimensions and margins might not scale well across different device sizes.
const style = StyleSheet.create({ musicFreeIcon: { - width: 50, - height: 50, - marginLeft: 20, - marginTop: 4, + width: theme.iconSizes.large, + height: theme.iconSizes.large, + margin: theme.spacing.medium, }, });src/stores/useMFsdk.ts (2)
18-23: Consider adding error handling for failed replacementsWhile the null check for empty array is good, consider handling cases where the replacement operation might fail.
replaceMFsdks: mf => { if (mf.length === 0) return; + try { set(s => ({ MFsdks: s.MFsdks.map(sdk => mf.find(v => v.srcUrl === sdk.srcUrl) ?? sdk), })); + } catch (error) { + console.error('Failed to replace MFsdks:', error); + } },
24-30: Consider consolidating storage operationsThe
addMFsdksandrmMFsdksmethods both handle storage operations before state updates. Consider extracting this pattern into a utility function to ensure consistent error handling.+const handleStorageOperation = async ( + operation: (paths: string[]) => Promise<void>, + sdks: MFsdk[], + paths: string[] +) => { + try { + await operation(paths); + return true; + } catch (error) { + console.error('Storage operation failed:', error); + return false; + } +}; addMFsdks: async mf => { if (mf.length === 0) return; - addMFsdks(mf.map(v => v.path)); + const success = await handleStorageOperation( + addMFsdks, + mf, + mf.map(v => v.path) + ); + if (!success) return; set(s => ({ MFsdks: getUniqObjects([...mf, ...s.MFsdks], sdk => sdk.srcUrl), })); },Also applies to: 31-39
src/components/setting/developer/View.tsx (2)
13-24: Consider extracting styles to a separate constantThe inline styles in HomeWrapper could be extracted to improve readability and reusability.
+const styles = { + wrapper: { + flex: 1, + }, +}; const HomeWrapper = ({ navigation }: NoxComponent.StackNavigationProps) => { const playerStyle = useNoxSetting(state => state.playerStyle); return ( <SelectDialogWrapper viewStyle={{ backgroundColor: playerStyle.customColors.maskedBackgroundColor, - flex: 1, + ...styles.wrapper, }} Children={p => Home({ ...p, navigation })} /> ); };
26-46: Consider adding screen options type safetyThe screen options could benefit from stronger typing to prevent potential configuration errors.
+type ScreenOptions = { + headerShown: boolean; +}; +const defaultScreenOptions: ScreenOptions = { + headerShown: false, +}; export default () => { return ( <Stack.Navigator> <Stack.Screen name={Route.HOME} component={HomeWrapper} - options={{ headerShown: false }} + options={defaultScreenOptions} /> <Stack.Screen name={Route.PLUGINS} component={PluginSettings} - options={{ headerShown: false }} + options={defaultScreenOptions} /> <Stack.Screen name={Route.MUSICFREE} component={MFSettings} - options={{ headerShown: false }} + options={defaultScreenOptions} /> </Stack.Navigator> ); };src/utils/fs.ts (1)
13-13: Consider using path.join for robust path constructionWhile the path construction works, using path.join would make it more robust across different platforms.
- `${fsdirs.DocumentDir}/${subfolder}/${filename}`, + path.join(fsdirs.DocumentDir, subfolder, filename),src/components/commonui/SearchBar.tsx (1)
9-13: Consider enhancing OnSearchProps interface documentationThe new interface provides good error handling and progress tracking capabilities. However, it would benefit from JSDoc comments explaining the purpose of each property.
+/** + * Props for the search operation + */ interface OnSearchProps { + /** The search query string */ v: string; + /** Callback to update search progress (0-1) */ progressEmitter: (v: number) => void; + /** Callback to show error/success messages */ setSnack: (v: SetSnack) => unknown; }src/components/setting/developer/plugins/musicfree/View.tsx (1)
73-80: Consider adding empty state and loading indicatorsThe FlashList should handle empty states and loading states appropriately.
<FlashList ref={scrollViewRef} data={MFsdks} renderItem={({ item }) => ( <RenderItem sdk={item} listRef={scrollViewRef} /> )} estimatedItemSize={100} + ListEmptyComponent={<Text>No SDKs installed</Text>} + onRefresh={onRefresh} + refreshing={isRefreshing} />src/utils/mediafetch/resolveURL.ts (1)
73-81: Consider memoizing the SDK lookupThe current implementation accesses state and performs SDK lookup on every call. Consider memoizing the SDK lookup or moving it outside the fallback function.
Here's a suggested approach:
+ const getMemoizedSDK = (source: string) => { + const mfsdks = useNoxSetting.getState().MFsdks; + return mfsdks.find(sdk => sdk.platform === source); + }; const fallback = () => { - const mfsdks = useNoxSetting.getState().MFsdks; - for (const mfsdk of mfsdks) { - if (mfsdk.platform === song.source) { - return mfsdk.resolveURL(song); - } - } + const sdk = getMemoizedSDK(song.source); + if (sdk) { + return sdk.resolveURL(song); + } return fetchBiliUrlPromise({ bvid, cid: String(cid), iOS, noBiliR128Gain }); };src/components/playlist/BiliSearch/BiliSearchbar.tsx (1)
46-46: Consider optimizing state subscriptionThe current implementation subscribes to the entire
MFsdksarray, which could cause unnecessary re-renders when any SDK is updated. Consider subscribing only to the length if that's all you need.Here's a suggested optimization:
- const mfsdks = useNoxSetting(state => state.MFsdks); + const hasMFSDKs = useNoxSetting(state => state.MFsdks.length > 0); const handleMenuPress = (event: GestureResponderEvent) => { - setShowMusicFree(mfsdks.length > 0); + setShowMusicFree(hasMFSDKs); setDialogOpen(true); setMenuCoords({ x: event.nativeEvent.pageX, y: event.nativeEvent.pageY, }); };Also applies to: 69-69
src/components/style.ts (1)
184-224: Consider extracting magic numbers into named constantsThe style definitions look well-structured, but there are some hardcoded values that could be extracted into named constants for better maintainability:
+const SKIN_ITEM_DIMENSIONS = { + SIZE: 72, + BORDER_RADIUS: 40 +}; export const ItemSelectStyles = StyleSheet.create({ // ... skinItemImage: { - width: 72, - height: 72, - borderRadius: 40, + width: SKIN_ITEM_DIMENSIONS.SIZE, + height: SKIN_ITEM_DIMENSIONS.SIZE, + borderRadius: SKIN_ITEM_DIMENSIONS.BORDER_RADIUS, }, // ... });src/stores/useApp.ts (1)
Line range hint
169-183: Consider adding loading state managementThe initialization of MFsdk and other resources could benefit from loading state management to improve user experience.
Consider adding a loading state:
interface NoxSetting extends BottomTabStore, APMUIStore, UIStore, PlaylistsStore, MFsdkStore { + isInitializing: boolean; // ... other properties } export const useNoxSetting = create<NoxSetting>((set, get, storeApi) => ({ // ... other implementations initPlayer: async val => { + set({ isInitializing: true }); try { // ... initialization logic + } finally { + set({ isInitializing: false }); } }, }));src/stores/useSlices.ts (1)
8-8: Consider adding documentation for the store's intended usage.The store is currently empty but appears to be a template. Consider adding JSDoc comments to document:
- The store's purpose
- Expected state structure
- How it should be extended
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (46)
.gitmodules(0 hunks)MusicFreePlugins(0 hunks)__tests__/musicfree/mfexample.js(1 hunks)__tests__/musicfree/musicfree.test.js(1 hunks)package.json(5 hunks)scripts/fixHTTP.py(1 hunks)src/components/commonui/SearchBar.tsx(1 hunks)src/components/playlist/BiliSearch/BiliSearchbar.tsx(2 hunks)src/components/playlist/BiliSearch/SearchMenu.tsx(2 hunks)src/components/setting/View.tsx(1 hunks)src/components/setting/appearances/SkinSearchbar.tsx(1 hunks)src/components/setting/appearances/SkinSettings.tsx(5 hunks)src/components/setting/developer/DeveloperSettings.tsx(3 hunks)src/components/setting/developer/View.tsx(1 hunks)src/components/setting/developer/enums.ts(1 hunks)src/components/setting/developer/plugins/View.tsx(1 hunks)src/components/setting/developer/plugins/musicfree/MusicFreeButton.tsx(1 hunks)src/components/setting/developer/plugins/musicfree/Searchbar.tsx(1 hunks)src/components/setting/developer/plugins/musicfree/View.tsx(1 hunks)src/components/setting/plugins/MusicFreeButton.tsx(0 hunks)src/components/style.ts(1 hunks)src/enums/Storage.ts(2 hunks)src/hooks/useLyricRN.ts(2 hunks)src/hooks/useNavigation.ts(1 hunks)src/localization/en/translation.json(1 hunks)src/localization/zhcn/translation.json(1 hunks)src/objects/Song.ts(1 hunks)src/objects/Storage.ts(2 hunks)src/stores/useAPMUI.ts(1 hunks)src/stores/useApp.ts(3 hunks)src/stores/useMFsdk.ts(1 hunks)src/stores/useSlices.ts(1 hunks)src/stores/useSnack.ts(1 hunks)src/types/media.d.ts(1 hunks)src/types/request.d.ts(1 hunks)src/utils/BiliSearch.ts(3 hunks)src/utils/Bilibili/biliCookies.ts(1 hunks)src/utils/ChromeStorage.ts(1 hunks)src/utils/fs.ts(2 hunks)src/utils/mediafetch/bilisearch.ts(2 hunks)src/utils/mediafetch/evalsdk.ts(1 hunks)src/utils/mediafetch/mfsdk.ts(0 hunks)src/utils/mediafetch/musicfree.ts(0 hunks)src/utils/mediafetch/resolveURL.ts(2 hunks)src/utils/mediafetch/ytbChannel.ytbi.ts(1 hunks)src/utils/mfsdk.ts(1 hunks)
💤 Files with no reviewable changes (5)
- MusicFreePlugins
- src/utils/mediafetch/mfsdk.ts
- .gitmodules
- src/components/setting/plugins/MusicFreeButton.tsx
- src/utils/mediafetch/musicfree.ts
✅ Files skipped from review due to trivial changes (3)
- scripts/fixHTTP.py
- src/utils/mediafetch/ytbChannel.ytbi.ts
- src/components/setting/developer/plugins/View.tsx
🧰 Additional context used
🪛 eslint
src/components/setting/developer/View.tsx
[error] 1-1: Resolve error: Cannot find module 'babel-plugin-module-resolver'
Require stack:
- /home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js
- /home/jailuser/git/node_modules/eslint-module-utils/resolve.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/rules/no-unresolved.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/index.js
- /home/jailuser/git/node_modules/@eslint/eslintrc/dist/eslintrc.cjs
at Module._resolveFilename (node:internal/modules/cjs/loader:1248:15)
at Module._load (node:internal/modules/cjs/loader:1074:27)
at TracingChannel.traceSync (node:diagnostics_channel:315:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:217:24)
at Module.require (node:internal/modules/cjs/loader:1339:12)
at require (node:internal/modules/helpers:135:16)
at Object. (/home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js:8:5)
at Module._compile (node:internal/modules/cjs/loader:1546:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1691:10)
at Module.load (node:internal/modules/cjs/loader:1317:32)
(import/namespace)
__tests__/musicfree/musicfree.test.js
[error] 1-1: Resolve error: Cannot find module 'babel-plugin-module-resolver'
Require stack:
- /home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js
- /home/jailuser/git/node_modules/eslint-module-utils/resolve.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/rules/no-unresolved.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/index.js
- /home/jailuser/git/node_modules/@eslint/eslintrc/dist/eslintrc.cjs
at Module._resolveFilename (node:internal/modules/cjs/loader:1248:15)
at Module._load (node:internal/modules/cjs/loader:1074:27)
at TracingChannel.traceSync (node:diagnostics_channel:315:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:217:24)
at Module.require (node:internal/modules/cjs/loader:1339:12)
at require (node:internal/modules/helpers:135:16)
at Object. (/home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js:8:5)
at Module._compile (node:internal/modules/cjs/loader:1546:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1691:10)
at Module.load (node:internal/modules/cjs/loader:1317:32)
(import/namespace)
[error] 1-1: Resolve error: Cannot find module 'babel-plugin-module-resolver'
Require stack:
- /home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js
- /home/jailuser/git/node_modules/eslint-module-utils/resolve.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/rules/no-unresolved.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/index.js
- /home/jailuser/git/node_modules/@eslint/eslintrc/dist/eslintrc.cjs
at Module._resolveFilename (node:internal/modules/cjs/loader:1248:15)
at Module._load (node:internal/modules/cjs/loader:1074:27)
at TracingChannel.traceSync (node:diagnostics_channel:315:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:217:24)
at Module.require (node:internal/modules/cjs/loader:1339:12)
at require (node:internal/modules/helpers:135:16)
at Object. (/home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js:8:5)
at Module._compile (node:internal/modules/cjs/loader:1546:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1691:10)
at Module.load (node:internal/modules/cjs/loader:1317:32)
(import/named)
[error] 4-4: 'test' is not defined.
(no-undef)
[error] 8-8: 'expect' is not defined.
(no-undef)
[error] 11-11: 'expect' is not defined.
(no-undef)
src/components/setting/developer/plugins/musicfree/Searchbar.tsx
[error] 1-1: Resolve error: Cannot find module 'babel-plugin-module-resolver'
Require stack:
- /home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js
- /home/jailuser/git/node_modules/eslint-module-utils/resolve.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/rules/no-unresolved.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/index.js
- /home/jailuser/git/node_modules/@eslint/eslintrc/dist/eslintrc.cjs
at Module._resolveFilename (node:internal/modules/cjs/loader:1248:15)
at Module._load (node:internal/modules/cjs/loader:1074:27)
at TracingChannel.traceSync (node:diagnostics_channel:315:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:217:24)
at Module.require (node:internal/modules/cjs/loader:1339:12)
at require (node:internal/modules/helpers:135:16)
at Object. (/home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js:8:5)
at Module._compile (node:internal/modules/cjs/loader:1546:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1691:10)
at Module.load (node:internal/modules/cjs/loader:1317:32)
(import/namespace)
src/components/setting/developer/plugins/musicfree/View.tsx
[error] 1-1: Resolve error: Cannot find module 'babel-plugin-module-resolver'
Require stack:
- /home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js
- /home/jailuser/git/node_modules/eslint-module-utils/resolve.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/rules/no-unresolved.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/index.js
- /home/jailuser/git/node_modules/@eslint/eslintrc/dist/eslintrc.cjs
at Module._resolveFilename (node:internal/modules/cjs/loader:1248:15)
at Module._load (node:internal/modules/cjs/loader:1074:27)
at TracingChannel.traceSync (node:diagnostics_channel:315:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:217:24)
at Module.require (node:internal/modules/cjs/loader:1339:12)
at require (node:internal/modules/helpers:135:16)
at Object. (/home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js:8:5)
at Module._compile (node:internal/modules/cjs/loader:1546:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1691:10)
at Module.load (node:internal/modules/cjs/loader:1317:32)
(import/namespace)
src/utils/mediafetch/evalsdk.ts
[error] 1-1: Resolve error: Cannot find module 'babel-plugin-module-resolver'
Require stack:
- /home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js
- /home/jailuser/git/node_modules/eslint-module-utils/resolve.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/rules/no-unresolved.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/index.js
- /home/jailuser/git/node_modules/@eslint/eslintrc/dist/eslintrc.cjs
at Module._resolveFilename (node:internal/modules/cjs/loader:1248:15)
at Module._load (node:internal/modules/cjs/loader:1074:27)
at TracingChannel.traceSync (node:diagnostics_channel:315:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:217:24)
at Module.require (node:internal/modules/cjs/loader:1339:12)
at require (node:internal/modules/helpers:135:16)
at Object. (/home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js:8:5)
at Module._compile (node:internal/modules/cjs/loader:1546:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1691:10)
at Module.load (node:internal/modules/cjs/loader:1317:32)
(import/namespace)
src/components/setting/appearances/SkinSearchbar.tsx
[error] 1-1: Resolve error: Cannot find module 'babel-plugin-module-resolver'
Require stack:
- /home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js
- /home/jailuser/git/node_modules/eslint-module-utils/resolve.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/rules/no-unresolved.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/index.js
- /home/jailuser/git/node_modules/@eslint/eslintrc/dist/eslintrc.cjs
at Module._resolveFilename (node:internal/modules/cjs/loader:1248:15)
at Module._load (node:internal/modules/cjs/loader:1074:27)
at TracingChannel.traceSync (node:diagnostics_channel:315:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:217:24)
at Module.require (node:internal/modules/cjs/loader:1339:12)
at require (node:internal/modules/helpers:135:16)
at Object. (/home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js:8:5)
at Module._compile (node:internal/modules/cjs/loader:1546:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1691:10)
at Module.load (node:internal/modules/cjs/loader:1317:32)
(import/namespace)
src/components/setting/developer/plugins/musicfree/MusicFreeButton.tsx
[error] 1-1: Resolve error: Cannot find module 'babel-plugin-module-resolver'
Require stack:
- /home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js
- /home/jailuser/git/node_modules/eslint-module-utils/resolve.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/rules/no-unresolved.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/index.js
- /home/jailuser/git/node_modules/@eslint/eslintrc/dist/eslintrc.cjs
at Module._resolveFilename (node:internal/modules/cjs/loader:1248:15)
at Module._load (node:internal/modules/cjs/loader:1074:27)
at TracingChannel.traceSync (node:diagnostics_channel:315:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:217:24)
at Module.require (node:internal/modules/cjs/loader:1339:12)
at require (node:internal/modules/helpers:135:16)
at Object. (/home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js:8:5)
at Module._compile (node:internal/modules/cjs/loader:1546:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1691:10)
at Module.load (node:internal/modules/cjs/loader:1317:32)
(import/namespace)
src/stores/useMFsdk.ts
[error] 1-1: Resolve error: Cannot find module 'babel-plugin-module-resolver'
Require stack:
- /home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js
- /home/jailuser/git/node_modules/eslint-module-utils/resolve.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/rules/no-unresolved.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/index.js
- /home/jailuser/git/node_modules/@eslint/eslintrc/dist/eslintrc.cjs
at Module._resolveFilename (node:internal/modules/cjs/loader:1248:15)
at Module._load (node:internal/modules/cjs/loader:1074:27)
at TracingChannel.traceSync (node:diagnostics_channel:315:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:217:24)
at Module.require (node:internal/modules/cjs/loader:1339:12)
at require (node:internal/modules/helpers:135:16)
at Object. (/home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js:8:5)
at Module._compile (node:internal/modules/cjs/loader:1546:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1691:10)
at Module.load (node:internal/modules/cjs/loader:1317:32)
(import/namespace)
__tests__/musicfree/mfexample.js
[error] 33-33: Expected an assignment or function call and instead saw an expression.
(@typescript-eslint/no-unused-expressions)
[error] 48-48: Expected an assignment or function call and instead saw an expression.
(@typescript-eslint/no-unused-expressions)
[error] 63-63: Do not access Object.prototype method 'hasOwnProperty' from target object.
(no-prototype-builtins)
[error] 82-82: 'e' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 82-82: Empty block statement.
(no-empty)
[error] 86-87: Expected an assignment or function call and instead saw an expression.
(@typescript-eslint/no-unused-expressions)
[error] 302-302: 'page' is defined but never used.
(@typescript-eslint/no-unused-vars)
src/utils/mfsdk.ts
[error] 1-1: Resolve error: Cannot find module 'babel-plugin-module-resolver'
Require stack:
- /home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js
- /home/jailuser/git/node_modules/eslint-module-utils/resolve.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/rules/no-unresolved.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/index.js
- /home/jailuser/git/node_modules/@eslint/eslintrc/dist/eslintrc.cjs
at Module._resolveFilename (node:internal/modules/cjs/loader:1248:15)
at Module._load (node:internal/modules/cjs/loader:1074:27)
at TracingChannel.traceSync (node:diagnostics_channel:315:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:217:24)
at Module.require (node:internal/modules/cjs/loader:1339:12)
at require (node:internal/modules/helpers:135:16)
at Object. (/home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js:8:5)
at Module._compile (node:internal/modules/cjs/loader:1546:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1691:10)
at Module.load (node:internal/modules/cjs/loader:1317:32)
(import/namespace)
src/stores/useSlices.ts
[error] 1-1: Resolve error: Cannot find module 'babel-plugin-module-resolver'
Require stack:
- /home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js
- /home/jailuser/git/node_modules/eslint-module-utils/resolve.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/rules/no-unresolved.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/index.js
- /home/jailuser/git/node_modules/@eslint/eslintrc/dist/eslintrc.cjs
at Module._resolveFilename (node:internal/modules/cjs/loader:1248:15)
at Module._load (node:internal/modules/cjs/loader:1074:27)
at TracingChannel.traceSync (node:diagnostics_channel:315:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:217:24)
at Module.require (node:internal/modules/cjs/loader:1339:12)
at require (node:internal/modules/helpers:135:16)
at Object. (/home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js:8:5)
at Module._compile (node:internal/modules/cjs/loader:1546:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1691:10)
at Module.load (node:internal/modules/cjs/loader:1317:32)
(import/namespace)
🪛 Gitleaks (8.21.2)
__tests__/musicfree/mfexample.js
97-97: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 Biome (1.9.4)
__tests__/musicfree/mfexample.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
[error] 63-63: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 81-81: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 86-86: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 131-135: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 132-132: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 153-153: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 156-156: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 158-158: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 159-159: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 160-160: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 162-162: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 475-475: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 485-485: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 52-52: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.
(lint/suspicious/useIsArray)
[error] 70-70: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.
(lint/suspicious/useIsArray)
src/stores/useSlices.ts
[error] 6-6: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
🪛 GitHub Check: CodeQL
__tests__/musicfree/mfexample.js
[failure] 97-97: Hard-coded credentials
The hard-coded value "f3ac5b086f3eab260520d8e3049561e6" is used as key.
🔇 Additional comments (35)
src/utils/mfsdk.ts (1)
12-19: Ensure external references to removed SDK paths are handled.
When removing SDK paths, confirm that no active references exist in the system. If so, handle them gracefully to prevent missing resource errors.
✅ Verification successful
Let me gather more information about how these SDK paths are used in the system.
The first command will help us understand how SDK paths are retrieved and used, while the second will look for any direct path assignments that might reference these SDKs.
Let me check one more aspect to understand how these SDKs are loaded and used in the system.
Based on the gathered information, I can now provide the final response:
The SDK removal implementation is safe and properly handled
The implementation in rmMFsdks correctly manages SDK removal by:
- Updating the stored paths in
StorageKeys.MFSDK_PATHS - Removing the physical files using
rmTxtFile - Properly updating the application state through the
useMFsdkstore - SDKs are loaded on-demand through
loadEvalPlugin, which gracefully handles failures
The system maintains consistency between the stored paths, physical files, and runtime state, preventing any dangling references.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the usage of removed SDK paths in the codebase.
rg -A 2 -B 2 'rmMFsdks'
Length of output: 2348
Script:
#!/bin/bash
# Check how MFsdk paths are used in the codebase
rg -A 5 "getMFsdk"
# Check for any direct path references
ast-grep --pattern 'path = $_'
Length of output: 1149
Script:
#!/bin/bash
# Check how these SDKs are loaded and used
rg -A 5 "loadEvalPlugin"
# Check the implementation of rmTxtFile
rg -A 3 "rmTxtFile"
Length of output: 3062
src/utils/mediafetch/evalsdk.ts (1)
57-71: Catch potential multi-page search expansions.
Your searchWrapper calls "search" with fixed parameters. If the original plugin supports paging, ensure you're not ignoring multi-page search results unintentionally.
src/components/setting/appearances/SkinSettings.tsx (4)
3-3: Minor note on platform imports.
Ensure that React Native's built-in modules offer everything needed. Avoid incomplete imports for animations or other cross-platform functionalities.
12-12: Potential circular dependencies?
Be mindful of import paths referencing each other across subfolders. If there's a risk of cyclical references, consider consolidating or reorganizing these modules.
136-136: Ref usage is correct.
Using a RefObject for FlashList is a good approach to handle layout animations and scrolling references.
184-184: Proper item sizing for improved performance.
With FlashList, "estimatedItemSize" is presumably correct. Keep an eye on metrics to ensure it’s not drastically under/overestimated, which could affect scroll performance.
src/components/setting/developer/DeveloperSettings.tsx (1)
12-13: Check correct usage of newly imported components.
Confirm that these imports still point to the correct modules after the reorganization.
Also applies to: 15-15, 20-21, 29-31
__tests__/musicfree/mfexample.js (2)
168-189: Check error handling for external HTTP request.
In case of network failure or malformed data, ensure the calling code has robust fallback or error messages.
326-359: Validate media source logic.
If no or unexpected data is returned from the third-party API, ensure the calling modules handle the null/undefined URL gracefully.
src/components/setting/developer/enums.ts (2)
1-5: Enums look good.
This is a clean approach to keep route constants organized. No immediate issues spotted.
7-19: Icons enum is well-defined.
No issues here.
__tests__/musicfree/musicfree.test.js (1)
4-12: Ensure Jest environment is properly configured.
You may see no-undef errors for 'test' and 'expect' if linting is not configured for Jest. Confirm your ESLint and jest config integration.
🧰 Tools
🪛 eslint
[error] 4-4: 'test' is not defined.
(no-undef)
[error] 8-8: 'expect' is not defined.
(no-undef)
[error] 11-11: 'expect' is not defined.
(no-undef)
src/utils/Bilibili/biliCookies.ts (3)
8-8: Enum usage for default parameter is clearer.
This improves consistency and maintainability.
11-11: Refactored getBiliJct function is concise.
No issues spotted.
16-16: Cookie header usage.
This approach is straightforward; just ensure the cookie value is always present.
src/enums/Storage.ts (2)
26-26: LGTM: Storage key for MusicFree paths
The new storage key follows the established naming convention and clearly indicates its purpose.
37-37: LGTM: New search option for MusicFree
The addition aligns with the PR's objective of restructuring the MusicFree plugin system.
src/stores/useMFsdk.ts (1)
7-13: LGTM: Well-defined store interface
The interface is clean, well-typed, and provides a comprehensive set of operations for managing MFsdk instances.
src/objects/Storage.ts (1)
Line range hint 25-42: Verify migration strategy for default setting changes
Several default settings have been changed from false to true:
- noCookieBiliSearch
- trackCoverArtCard
- karaokeLyrics
- memoryEfficiency
These changes could affect existing users' experience. Please ensure there's a migration strategy in place to handle these changes gracefully.
✅ Verification successful
Let me gather more information about the storage implementation and any potential migrations.
Let me try a different approach to check the storage implementation and any migration-related code.
Let me check one more aspect to see how settings are loaded and applied in the application.
Based on the gathered information, I can now provide the final response:
Default settings changes are handled safely through persistence layer
The changes to default settings are safe because:
- The app uses a layered approach to settings through
AsyncStorage:DefaultSettingserves as the base layer- Persisted settings are loaded and merged on top via
ChromeStorage.ts - An
OverrideSettinglayer exists for runtime overrides
- Existing user settings are preserved as they are stored separately and merged with defaults during initialization
- The changes only affect the base defaults, which are only used for new installations or when a setting is not found in storage
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for migration code handling these setting changes
rg -A 5 "migration|upgrade|setting.*update" src/
Length of output: 464
Script:
#!/bin/bash
# Check storage implementation and initialization
ast-grep --pattern 'class Storage {
$$$
}'
# Check for any storage version or initialization code
rg -A 10 "initialize|init.*storage|version" src/objects/Storage.ts
# Look for any code that handles storage updates
rg -A 5 "storage.*update|update.*storage" src/
Length of output: 164
Script:
#!/bin/bash
# Get the full content of Storage.ts to understand the implementation
cat src/objects/Storage.ts
# Check for any storage-related changes in recent commits
rg -A 5 "Storage" src/migrations/ || echo "No migrations directory found"
# Look for any persistence or storage loading code
rg -A 5 "load.*settings|save.*settings|persist.*settings" src/
Length of output: 1919
Script:
#!/bin/bash
# Look for settings initialization and loading in the codebase
rg -A 10 "DefaultSetting|OverrideSetting" src/
# Check for any persistence mechanism
rg -A 10 "AsyncStorage|localStorage|Storage\.set" src/
Length of output: 11471
src/types/media.d.ts (1)
7-7: Verify the impact of removing MUSICFREE from SongSource type
This is a breaking change that could affect existing code using the MUSICFREE source type. Please ensure all consumers of this type have been updated accordingly.
Consider adding a comment documenting this breaking change and migration steps for downstream consumers.
src/stores/useAPMUI.ts (1)
16-17: Verify SearchOptions enum values after MUSICFREE removal
The searchOption type has been updated to remove MUSICFREE. Please ensure:
- The SearchOptions enum has been updated accordingly
- The default value BILIBILI is still valid
- The saveDefaultSearch function handles the updated type
src/objects/Song.ts (1)
28-28: Verify removal of MUSICFREE source type
The simplification of the source property type aligns with the PR's objective of restructuring the MusicFree plugin system. However, we should verify that this change doesn't break existing code.
✅ Verification successful
Let me check the actual usage of MUSICFREE in these files to understand if they're related to the source type.
Based on the search results, I can now generate the final response as I have enough context. The MUSICFREE references found are not related to the Source type in Song.ts, but rather to enum values, route names, and plugin-specific functionality. Let me check the Source type definition to be completely certain.
No issues with Source type change - MUSICFREE was never a valid Source enum value
The verification shows that the MUSICFREE type was never part of the Source enum in MediaFetch.ts. The Source enum only contains 'biliaudio', 'bilivideo', and 'steriatk' as valid values. The MUSICFREE references found in the codebase are used in different contexts (routes, plugin names, search options) and are not related to the Song's source property type. This change is safe and correct.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining MUSICFREE type usage that might need updating
rg -l "MUSICFREE" --type ts
# Search for direct assignments of MUSICFREE to source property
ast-grep --pattern 'source: MUSICFREE'
Length of output: 398
Script:
#!/bin/bash
# Check content of files containing MUSICFREE
rg "MUSICFREE" -C 2 --type ts
# Also search for any Source type usage
rg "Source\s*[:{]" --type ts
Length of output: 3774
Script:
#!/bin/bash
# Check Source type definition and usage in objects/Song.ts context
rg "Source" -C 3 src/enums/MediaFetch.ts
rg "Source" -C 3 src/objects/Song.ts
Length of output: 575
src/types/request.d.ts (1)
98-98: LGTM: Improved type safety with NoxMedia namespace
The update to use explicit NoxMedia namespace types improves type safety and consistency.
src/stores/useSnack.ts (1)
Line range hint 5-11: LGTM: Improved modularity with exported interface
The addition of the export keyword to SetSnack interface improves code modularity and reusability, particularly for its use in the SearchBar component.
src/components/playlist/BiliSearch/SearchMenu.tsx (1)
35-35: Verify MusicFree search option handling
The changes to setDefaultSearch and MusicFree menu item look correct, but we should verify that all callers of this function have been updated to use SearchOptions.MUSICFREE.
Also applies to: 76-77
✅ Verification successful
All callers of setDefaultSearch are using SearchOptions enum correctly
The verification shows that:
- All calls to
setDefaultSearchconsistently use theSearchOptionsenum, includingSearchOptions.MUSICFREE - The
SearchOptionsenum is used properly throughout the codebase for MusicFree handling - The only raw "MUSICFREE" string usage is in a test file and refers to a different constant, unrelated to the search options
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining usage of MUSICFREE type or direct string literals
rg -l "MUSICFREE\." --type ts
rg -l "'MUSICFREE'" --type ts
rg -l "\"MUSICFREE\"" --type ts
Length of output: 136
Script:
#!/bin/bash
# Let's check the actual usage context of setDefaultSearch function
ast-grep --pattern 'setDefaultSearch($_)'
# Also search for SearchOptions enum usage
ast-grep --pattern 'SearchOptions.$_'
# And check the test file content that matched MUSICFREE
cat __tests__/mediafetch/mfsdk.test.ts
Length of output: 2832
src/hooks/useLyricRN.ts (1)
26-26: Verify path handling consistency and data migration
The change from 'lrc/' to 'lrc' directory path needs careful consideration:
- Ensure all other file operations use consistent path format
- Consider existing files in the old path format
Consider implementing a path normalization utility to ensure consistent path handling across the application.
Also applies to: 44-44
✅ Verification successful
Based on the search results, I can now generate the final response:
Path handling is consistent across the codebase
The codebase shows consistent path handling:
- All file operations use the same pattern where subfolder is passed as a separate parameter to
readTxtFile/writeTxtFile - The implementation in
fs.tscorrectly joins paths using${fsdirs.DocumentDir}/${subfolder}/${filename} - No other instances of hardcoded 'lrc/' paths were found that would need updates
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other instances of lrc path usage
rg -A 2 "('|\")lrc/?" --type ts
# Search for file operations that might need similar updates
rg -A 2 "(readTxtFile|writeTxtFile)" --type ts
Length of output: 2538
src/utils/mediafetch/bilisearch.ts (1)
4-4: LGTM! Good improvement in cookie management
The change to use BILICOOKIES.SESSDATA enum instead of string literals is a good practice that centralizes cookie key management and reduces maintenance overhead.
Also applies to: 22-22
src/components/style.ts (1)
208-213: Review negative margins usage
The use of negative margins could lead to layout inconsistencies across different screen sizes and devices. Consider using positive margins/padding with flex layout instead.
src/components/setting/View.tsx (1)
8-8: LGTM! Good structural improvement
The reorganization of developer settings into a dedicated folder improves code organization.
src/stores/useApp.ts (1)
22-23: LGTM! Clean integration of MFsdk store
The integration of MFsdk store through interface extension and import is well-structured.
Also applies to: 29-30
package.json (1)
34-35: Verify compatibility with downgraded CLI platform packages
The React Native CLI platform packages for Android and iOS have been downgraded from 16.0.2 to 15.1.3. This could potentially impact the build process.
✅ Verification successful
CLI platform packages downgrade is safe to proceed
The downgrade from 16.0.2 to 15.1.3 for React Native CLI platform packages appears safe because:
- The project uses standard React Native CLI commands without custom platform-specific configurations
- The metro configuration is using the default setup with Sentry integration
- The react-native.config.js is minimal without platform-specific customizations
- Build configurations in android/build.gradle use standard React Native Gradle plugin setup
- The project is on React Native 0.76.5, which is compatible with CLI platform packages version 15.x
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any breaking changes or known issues between versions
# Check package.json for React Native version compatibility
rg -A 5 '"react-native":\s*"[^"]*"' package.json
# Check for any migration guides or breaking changes in the repository
rg -i -A 5 'migration|breaking.?changes?' .github/ docs/ CHANGELOG.md README.md
Length of output: 495
Script:
#!/bin/bash
# Let's check the React Native version and CLI compatibility by examining the repository
# Check if there are any direct dependencies on these CLI packages in the code
rg -l "@react-native-community/cli-platform-" --type js --type ts
# Check for any build configurations that might be affected
fd "build.gradle|project.pbxproj|metro.config.js"
# Check for any CLI-related configurations
fd "react-native.config.js"
# Look for any CLI-related scripts in package.json
rg '"scripts":\s*{' -A 20 package.json
Length of output: 397
Script:
#!/bin/bash
# Let's check the content of key configuration files
# Check metro config for any CLI-specific settings
cat metro.config.js
# Check react-native config for any platform-specific settings
cat react-native.config.js
# Check package.json scripts (with fixed regex)
rg '"scripts":' -A 20 package.json
# Check the root build.gradle for any CLI-related configurations
cat android/build.gradle
Length of output: 2306
src/utils/ChromeStorage.ts (1)
56-57: LGTM! Type signature update is consistent
The removal of MUSICFREE type from saveDefaultSearch is consistent with the broader refactoring of the MusicFree plugin system.
src/localization/zhcn/translation.json (1)
270-270: LGTM! Warning message is consistent with English translation.
The warning message change from "启用 MusicFree 插件" to "不要使用 后果自负" aligns with the English translation and effectively communicates the risks to users.
src/localization/en/translation.json (1)
256-256: LGTM! Warning message effectively communicates the risk.
The warning message change from "Enable MusicFree plugins" to "Do NOT use, uve been warned." is clear and consistent with the Chinese translation.
src/stores/useSlices.ts (1)
1-10: Verify integration with MFsdk store.
This template appears to be part of the MusicFree SDK restructuring. Consider documenting how this slice template integrates with the new MFsdk store mentioned in the changes.
Run this script to understand the relationship with other stores:
🧰 Tools
🪛 Biome (1.9.4)
[error] 6-6: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
🪛 eslint
[error] 1-1: Resolve error: Cannot find module 'babel-plugin-module-resolver'
Require stack:
- /home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js
- /home/jailuser/git/node_modules/eslint-module-utils/resolve.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/rules/no-unresolved.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/index.js
- /home/jailuser/git/node_modules/@eslint/eslintrc/dist/eslintrc.cjs
at Module._resolveFilename (node:internal/modules/cjs/loader:1248:15)
at Module._load (node:internal/modules/cjs/loader:1074:27)
at TracingChannel.traceSync (node:diagnostics_channel:315:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:217:24)
at Module.require (node:internal/modules/cjs/loader:1339:12)
at require (node:internal/modules/helpers:135:16)
at Object. (/home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js:8:5)
at Module._compile (node:internal/modules/cjs/loader:1546:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1691:10)
at Module.load (node:internal/modules/cjs/loader:1317:32)
(import/namespace)
| export const fetchMFsdk = async (url: string): Promise<MFsdk[]> => { | ||
| try { | ||
| const res = await bFetch(url); | ||
| const text = await res.text(); | ||
| try { | ||
| const json = JSON.parse(text) as { plugins: { url: string }[] }; | ||
| const sdks = await Promise.all(json.plugins.map(p => fetchMFsdk(p.url))); | ||
| return sdks.flat(); | ||
| } catch { | ||
| // do nothing | ||
| } | ||
| const loadedSDK = loadEvalPlugin(text, url); | ||
| const sdkLocalPath = `${loadedSDK.platform}.${loadedSDK.version}.js`; | ||
| loadedSDK.path = sdkLocalPath; | ||
| writeTxtFile(sdkLocalPath, [text], mfsdkSubFolder); | ||
| return [loadedSDK]; | ||
| } catch (e) { | ||
| logger.warn(`[mfsdk] failed to fetch and parse ${url}: ${e}`); | ||
| } | ||
| return []; | ||
| }; |
There was a problem hiding this comment.
💡 Codebase verification
Add recursion depth limit to prevent potential stack overflow
The current implementation of fetchMFsdk has no safeguards against deep recursion. While the code handles nested plugin structures, it could potentially lead to a stack overflow or performance issues if malicious or poorly structured JSON is provided. Consider adding a depth parameter with a reasonable limit (e.g., 3-5 levels).
Example locations that need attention:
src/utils/mfsdk.ts:42: Add depth parameter tofetchMFsdkfunctionsrc/utils/mfsdk.ts:48: Check depth limit before recursive calls
🔗 Analysis chain
Gracefully handle deeply nested JSON structures.
In line 48, you recursively call fetchMFsdk on nested plugin URLs. While it looks correct, ensure there's no infinite recursion or extremely deep nesting scenario that might degrade performance.
Let me analyze the code further to check for any safeguards against deep recursion.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
#!/bin/bash
# Check if there's any potential unbounded recursion in fetchMFsdk usage.
rg -A 5 -B 5 'fetchMFsdk'
Length of output: 4885
Script:
#!/bin/bash
# Check the structure of plugin files to understand nesting patterns
ast-grep --pattern 'const json = JSON.parse($_) as { plugins: { url: string }[] }'
Length of output: 177
| export const loadEvalPlugin = (plugin: string, path = 'MEMORY'): MFsdk => { | ||
| const env = { | ||
| getUserVariables: () => ({}), | ||
| os: 'android', | ||
| }; | ||
|
|
||
| const _module: any = { exports: {} }; | ||
| let _instance: any; | ||
| // eslint-disable-next-line no-new-func | ||
| _instance = Function(` | ||
| 'use strict'; | ||
| return function(require, __musicfree_require, module, exports, console, env, URL) { | ||
| ${plugin} | ||
| } | ||
| `)()( | ||
| _require, | ||
| _require, | ||
| _module, | ||
| _module.exports, | ||
| _console, | ||
| env, | ||
| URL, | ||
| ); | ||
| if (_module.exports.default) { | ||
| _instance = _module.exports.default; | ||
| } else { | ||
| _instance = _module.exports; | ||
| } | ||
| return { | ||
| ..._instance, | ||
| path, | ||
| regexFetch: searchWrapper(_instance.search, _instance), | ||
| resolveURL: resolveURLWrapper(_instance.getMediaSource, _instance), | ||
| }; | ||
| }; |
There was a problem hiding this comment.
Ensure plugin code is sandboxed and safe.
You dynamically evaluate plugin code using Function(). Double-check that no malicious code can break out of this environment. You might want to implement additional sandbox checks or constraints.
| @@ -224,7 +200,7 @@ const Home = ({ | |||
| <SettingListItem | |||
| icon={Icons.plugins} | |||
| settingName="PluginsSetting" | |||
| onPress={() => navigation.navigate(VIEW.PLUGINS)} | |||
| onPress={() => navigation.navigate(Route.PLUGINS)} | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Route refactor alignment.
You are navigating with Route.PLUGINS. Ensure that old references to VIEW.PLUGINS or other enumerations have been fully removed to prevent mismatch.
#!/bin/bash
# Searching entire codebase for references to old "VIEW.PLUGINS"
rg 'VIEW\.PLUGINS'| const navigate2 = (route: unknown) => | ||
| navigationGlobal.navigate(route as never); |
There was a problem hiding this comment.
Consider improving type safety of navigate2
The use of unknown type with casting to never bypasses TypeScript's type checking. This could lead to runtime errors if invalid routes are passed.
Consider using a proper type for the route parameter:
- const navigate2 = (route: unknown) =>
- navigationGlobal.navigate(route as never);
+ const navigate2 = (route: NoxRoutes) =>
+ navigationGlobal.navigate(route);Committable suggestion skipped: line range outside the PR's diff.
| onSearch={async ({ v, setSnack }) => { | ||
| try { | ||
| addMFsdks(await fetchMFsdk(v)); | ||
| } catch (e) { | ||
| logger.warn(`[mfsdk] failed to search ${v}: ${e}`); | ||
| setSnack({ | ||
| snackMsg: { success: t('MFSDK.SearchFailMsg') }, | ||
| }); | ||
| } | ||
| }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider improving error handling and UX
The error handling could be more informative, and the search operation lacks loading state feedback.
Consider these improvements:
<SearchBar
defaultSearchText=""
+ loading={isLoading}
onSearch={async ({ v, setSnack }) => {
+ if (!v.trim()) {
+ setSnack({
+ snackMsg: { error: t('MFSDK.EmptySearchMsg') },
+ });
+ return;
+ }
+ setIsLoading(true);
try {
addMFsdks(await fetchMFsdk(v));
+ setSnack({
+ snackMsg: { success: t('MFSDK.SearchSuccessMsg') },
+ });
} catch (e) {
logger.warn(`[mfsdk] failed to search ${v}: ${e}`);
setSnack({
- snackMsg: { success: t('MFSDK.SearchFailMsg') },
+ snackMsg: { error: t('MFSDK.SearchFailMsg', { error: e.message }) },
});
+ } finally {
+ setIsLoading(false);
}
}}Committable suggestion skipped: line range outside the PR's diff.
| <IconButton | ||
| icon="sync" | ||
| style={styles.deleteButton} | ||
| onPress={() => fetchMFsdk(sdk.srcUrl).then(replaceMFsdks)} | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add loading state and error handling for SDK refresh
The refresh operation should show loading state and handle potential fetch failures.
+const [isRefreshing, setIsRefreshing] = React.useState(false);
<IconButton
icon="sync"
style={styles.deleteButton}
+ disabled={isRefreshing}
onPress={() => {
+ setIsRefreshing(true);
fetchMFsdk(sdk.srcUrl)
.then(replaceMFsdks)
+ .catch(error => {
+ console.error('Failed to refresh SDK:', error);
+ // Consider showing a user-friendly error message
+ })
+ .finally(() => setIsRefreshing(false));
}}
/>Committable suggestion skipped: line range outside the PR's diff.
| const fallback = () => { | ||
| const mfsdks = useNoxSetting.getState().MFsdks; | ||
| for (const mfsdk of mfsdks) { | ||
| if (mfsdk.platform === song.source) { | ||
| return mfsdk.resolveURL(song); | ||
| } | ||
| } | ||
| return result; | ||
| } | ||
| return fetchBiliUrlPromise({ bvid, cid: String(cid), iOS, noBiliR128Gain }); | ||
| }; |
There was a problem hiding this comment.
Add error handling for SDK resolution
The fallback mechanism needs to handle potential failures in SDK URL resolution. Consider wrapping the SDK call in a try-catch block.
Here's a suggested improvement:
const fallback = () => {
const mfsdks = useNoxSetting.getState().MFsdks;
for (const mfsdk of mfsdks) {
if (mfsdk.platform === song.source) {
- return mfsdk.resolveURL(song);
+ try {
+ return mfsdk.resolveURL(song);
+ } catch (error) {
+ logger.error(`Failed to resolve URL using ${mfsdk.platform} SDK:`, error);
+ continue;
+ }
}
}
return fetchBiliUrlPromise({ bvid, cid: String(cid), iOS, noBiliR128Gain });
};Committable suggestion skipped: line range outside the PR's diff.
| @@ -162,6 +166,7 @@ export const useNoxSetting = create<NoxSetting>((set, get, storeApi) => ({ | |||
| }); | |||
| const initializedPlayerSetting = val.settings; | |||
| set({ | |||
| MFsdks: await initMFsdk(), | |||
There was a problem hiding this comment.
Add error handling for MFsdk initialization
The initMFsdk() call should include error handling to manage initialization failures gracefully.
Consider wrapping the initialization in a try-catch block:
- MFsdks: await initMFsdk(),
+ MFsdks: await initMFsdk().catch(error => {
+ console.error('Failed to initialize MFsdk:', error);
+ return [];
+ }),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| MFsdks: await initMFsdk(), | |
| MFsdks: await initMFsdk().catch(error => { | |
| console.error('Failed to initialize MFsdk:', error); | |
| return []; | |
| }), |
| case SearchOptions.MUSICFREE: { | ||
| const songLists = await Promise.all( | ||
| useNoxSetting | ||
| .getState() | ||
| .MFsdks.map(sdk => sdk.regexFetch({ url: input })), | ||
| ); | ||
| results.songList = songLists.map(v => v.songList).flat(); | ||
| break; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider error handling for individual SDK failures
The current implementation using Promise.all will fail entirely if any single SDK fetch fails. Consider using Promise.allSettled to handle partial failures gracefully.
- const songLists = await Promise.all(
- useNoxSetting
- .getState()
- .MFsdks.map(sdk => sdk.regexFetch({ url: input })),
- );
- results.songList = songLists.map(v => v.songList).flat();
+ const songLists = await Promise.allSettled(
+ useNoxSetting
+ .getState()
+ .MFsdks.map(sdk => sdk.regexFetch({ url: input })),
+ );
+ results.songList = songLists
+ .filter((result): result is PromiseFulfilledResult<any> => result.status === 'fulfilled')
+ .map(result => result.value.songList)
+ .flat();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case SearchOptions.MUSICFREE: { | |
| const songLists = await Promise.all( | |
| useNoxSetting | |
| .getState() | |
| .MFsdks.map(sdk => sdk.regexFetch({ url: input })), | |
| ); | |
| results.songList = songLists.map(v => v.songList).flat(); | |
| break; | |
| } | |
| case SearchOptions.MUSICFREE: { | |
| const songLists = await Promise.allSettled( | |
| useNoxSetting | |
| .getState() | |
| .MFsdks.map(sdk => sdk.regexFetch({ url: input })), | |
| ); | |
| results.songList = songLists | |
| .filter((result): result is PromiseFulfilledResult<any> => result.status === 'fulfilled') | |
| .map(result => result.value.songList) | |
| .flat(); | |
| break; | |
| } |
| // eslint-disable-next-line @typescript-eslint/no-empty-object-type | ||
| export interface Slice {} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using a type alias instead of an empty interface.
An empty interface is equivalent to {}. Consider using a type alias for better TypeScript practices:
-// eslint-disable-next-line @typescript-eslint/no-empty-object-type
-export interface Slice {}
+export type Slice = Record<string, unknown>;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // eslint-disable-next-line @typescript-eslint/no-empty-object-type | |
| export interface Slice {} | |
| export type Slice = Record<string, unknown>; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 6-6: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores