-
Notifications
You must be signed in to change notification settings - Fork 62
feat(scraper): add ArcaneScraper and transformer for arcanes #714
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
Conversation
WalkthroughThe pull request updates the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🪧 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 (
|
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.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🧹 Nitpick comments (1)
build/parser.mjs (1)
451-469: Implementation of arcane type determination.The
addTypemethod now checks for arcanes in the wikia data and sets the appropriate type. The implementation is logical and follows the existing code patterns.Consider using an early return pattern for better readability:
addType(item, data) { if (item.parent) return; const arcane = data.wikia.arcanes.find((entry) => entry.name === item.name); if (arcane) { item.type = `${arcane.type} Arcane`; return; } - else { - // eslint-disable-next-line no-restricted-syntax - for (const type of types) { - const contains = type.regex - ? new RegExp(type.id, 'ig').test(item.uniqueName) - : item.uniqueName.includes(type.id); - if (contains) { - if (type.append) item.type = `${item.type}${type.name}`; - else item.type = type.name; - // if (item.type !== type.name) console.error(`${item.name} didn't update types`) - break; - } - } - } + // eslint-disable-next-line no-restricted-syntax + for (const type of types) { + const contains = type.regex + ? new RegExp(type.id, 'ig').test(item.uniqueName) + : item.uniqueName.includes(type.id); + if (contains) { + if (type.append) item.type = `${item.type}${type.name}`; + else item.type = type.name; + // if (item.type !== type.name) console.error(`${item.name} didn't update types`) + break; + } + } // No type assigned? Add 'Misc'. if (!item.type) { // ...
🛑 Comments failed to post (1)
build/tradable.mjs (1)
35-35:
⚠️ Potential issueFix array formatting to pass linting.
The CI pipeline is failing due to a formatting issue with this line. Each array element should be on its own line according to the project's style guide.
Apply this fix to resolve the linting error:
-const tradableTypes = ['Captura', 'Cut Gem', 'Fish', 'Focus Lens', 'Relic', 'Upgrades', ...tradableArcanes, ...tradableMods]; +const tradableTypes = [ + 'Captura', + 'Cut Gem', + 'Fish', + 'Focus Lens', + 'Relic', + 'Upgrades', + ...tradableArcanes, + ...tradableMods, +];📝 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.const tradableTypes = [ 'Captura', 'Cut Gem', 'Fish', 'Focus Lens', 'Relic', 'Upgrades', ...tradableArcanes, ...tradableMods, ];🧰 Tools
🪛 ESLint
[error] 35-35: Replace
'Captura',·'Cut·Gem',·'Fish',·'Focus·Lens',·'Relic',·'Upgrades',·...tradableArcanes,·...tradableModswith⏎··'Captura',⏎··'Cut·Gem',⏎··'Fish',⏎··'Focus·Lens',⏎··'Relic',⏎··'Upgrades',⏎··...tradableArcanes,⏎··...tradableMods,⏎(prettier/prettier)
🪛 GitHub Check: Lint
[failure] 35-35:
Replace'Captura',·'Cut·Gem',·'Fish',·'Focus·Lens',·'Relic',·'Upgrades',·...tradableArcanes,·...tradableModswith⏎··'Captura',⏎··'Cut·Gem',⏎··'Fish',⏎··'Focus·Lens',⏎··'Relic',⏎··'Upgrades',⏎··...tradableArcanes,⏎··...tradableMods,⏎🪛 GitHub Actions: Pull Request Checks
[error] 35-35: Prettier formatting check failed. Replace
'Captura', 'Cut Gem', 'Fish', 'Focus Lens', 'Relic', 'Upgrades', ...tradableArcanes, ...tradableModswith⏎ 'Captura',⏎ 'Cut Gem',⏎ 'Fish',⏎ 'Focus Lens',⏎ 'Relic',⏎ 'Upgrades',⏎ ...tradableArcanes,⏎ ...tradableMods,⏎.
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
build/parser.mjs (2)
451-469: Code properly identifies arcane types from wiki data.The implementation correctly checks for arcanes in the wikia data and assigns the appropriate type. This addresses the core functionality needed for the ArcaneScraper.
One optimization suggestion:
- // eslint-disable-next-line no-restricted-syntax - for (const type of types) { - const contains = type.regex - ? new RegExp(type.id, 'ig').test(item.uniqueName) - : item.uniqueName.includes(type.id); - if (contains) { - if (type.append) item.type = `${item.type}${type.name}`; - else item.type = type.name; - // if (item.type !== type.name) console.error(`${item.name} didn't update types`) - break; - } - } + const matchedType = types.find((type) => { + const contains = type.regex + ? new RegExp(type.id, 'ig').test(item.uniqueName) + : item.uniqueName.includes(type.id); + return contains; + }); + + if (matchedType) { + if (matchedType.append) item.type = `${item.type}${matchedType.name}`; + else item.type = matchedType.name; + }
1002-1013: Fix the JSDoc comment for the arcane wiki data method.The method comment incorrectly describes this as adding data for mods rather than arcanes.
/** - * Add additional data for mods from the wiki + * Add additional data for arcanes from the wiki * @param {Item} item mod to append wikia data to - * @param {WikiaArcane} wikiaItem to pull data from + * @param {WikiaArcane} wikiaItem arcane data to pull from */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
build/parser.mjs(6 hunks)build/scraper.mjs(4 hunks)build/tradable.mjs(2 hunks)build/wikia/WikiaDataScraper.mjs(0 hunks)build/wikia/scrapers/ArcaneScraper.mjs(1 hunks)build/wikia/transformers/transformArcanes.mjs(1 hunks)index.d.ts(2 hunks)
💤 Files with no reviewable changes (1)
- build/wikia/WikiaDataScraper.mjs
🚧 Files skipped from review as they are similar to previous changes (4)
- build/wikia/scrapers/ArcaneScraper.mjs
- build/scraper.mjs
- build/wikia/transformers/transformArcanes.mjs
- index.d.ts
🧰 Additional context used
🧠 Learnings (1)
build/parser.mjs (1)
Learnt from: justsomelegs
PR: WFCD/warframe-items#714
File: build/parser.mjs:929-931
Timestamp: 2025-03-29T16:32:58.039Z
Learning: In the Parser class, category names are normalized with `.toLowerCase()` before being used in switch statements, allowing case-insensitive matching.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build
🔇 Additional comments (4)
build/tradable.mjs (2)
4-17: Good addition of tradable arcane types.This comprehensive list of arcane types ensures that all arcane variants will be properly marked as tradable in the system. The naming pattern follows the established format seen with mods (
${type} Mod).
35-44: LGTM! Implementation follows the established pattern.The changes to
tradableTypescorrectly integrate the arcane types using the spread operator, matching how the tradable mods are handled.build/parser.mjs (2)
929-931: Support for arcanes category in the switch statement.The switch statement properly includes the 'arcanes' case with a call to the new
addArcaneWikiaDatamethod. As noted in the retrieved learnings, the.toLowerCase()on the category name ensures case-insensitive matching.
196-196: Data parameter propagation is correctly implemented.The
dataparameter is now passed toaddTypein both thefilterandquickFiltermethods, ensuring arcane type resolution works throughout the processing pipeline.Also applies to: 233-233
Introduce a new scraper class `ArcaneScraper` and a corresponding transformer `transformArcanes` to handle scraping and transforming arcane data from the Warframe wiki.
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
build/parser.mjs(6 hunks)build/scraper.mjs(4 hunks)build/tradable.mjs(2 hunks)build/wikia/WikiaDataScraper.mjs(0 hunks)build/wikia/scrapers/ArcaneScraper.mjs(1 hunks)build/wikia/transformers/transformArcanes.mjs(1 hunks)index.d.ts(2 hunks)
💤 Files with no reviewable changes (1)
- build/wikia/WikiaDataScraper.mjs
🚧 Files skipped from review as they are similar to previous changes (5)
- build/wikia/scrapers/ArcaneScraper.mjs
- build/wikia/transformers/transformArcanes.mjs
- build/scraper.mjs
- build/tradable.mjs
- index.d.ts
🧰 Additional context used
🧠 Learnings (1)
build/parser.mjs (1)
Learnt from: justsomelegs
PR: WFCD/warframe-items#714
File: build/parser.mjs:929-931
Timestamp: 2025-03-29T16:32:58.039Z
Learning: In the Parser class, category names are normalized with `.toLowerCase()` before being used in switch statements, allowing case-insensitive matching.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build
🔇 Additional comments (4)
build/parser.mjs (4)
929-931: Switch statement correctly handles Arcanes categoryThe switch statement now handles the 'arcanes' category by calling the new
addArcaneWikiaDatamethod. As noted in the retrieved learnings, category names are normalized with.toLowerCase()before being used in switch statements, so this will correctly match any case variation of "Arcanes".
1002-1013: NewaddArcaneWikiaDatamethod is well-structuredThe method follows the same pattern as other similar methods in the class (
addModWikiaData,addWeaponWikiaData, etc.), which maintains code consistency. The JSDoc documentation is also complete.The method correctly sets the item's thumbnail, URL, transmutability, and type from the Wikia data.
196-196: Update offiltermethod to pass data parameterThe method now correctly passes the
dataparameter to theaddTypemethod, ensuring arcanes can be properly typed during the filtering process.
233-233: Update ofquickFiltermethod to pass data parameterSimilar to the
filtermethod update, thequickFiltermethod now correctly passes thedataparameter to theaddTypemethod.
|
🎉 This PR is included in version 1.1268.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Introduce a new scraper class
ArcaneScraperand a corresponding transformertransformArcanesto handle scraping and transforming arcane data from the Warframe wiki.What did you fix?
Add the ability for Arcanes to have types corresponding to the Item it slots onto by introducing a wiki scraper for them.
Reproduction steps
Evidence/screenshot/link to line
Considerations
Summary by CodeRabbit