-
Notifications
You must be signed in to change notification settings - Fork 874
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
Expose all assessors and word complexity helpers #21337
base: trunk
Are you sure you want to change the base?
Conversation
…e-all-assessors-in-indexjs
Pull Request Test Coverage Report for Build 763724a873c65f1d00a803286873d46876d0e93aDetails
💛 - Coveralls |
… assessors and word complexity helpers and configs
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.
Nice work! 🎉 Just a few small things to consider before we can merge 👍
import Assessor from "../../../../src/scoring/assessors/collectionPages/seoAssessor.js"; | ||
import Paper from "../../../../src/values/Paper.js"; |
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.
In a few of these files, the import end with .js
, in others it's without .js
. Let's settle on one way for all of these (I guess without .js
).
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.
I found this info on Stackoverflow where the way I understand it is that using file extension is actually better because tools like Webpack can resolve it immediately, except for index.js
I guess where we don't need to add the extension. What do you think?
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.
OK, let's use the extension 👍
import SEOAssessor from "../../src/scoring/seoAssessor"; | ||
import contentAssessor from "../../src/scoring/contentAssessor"; | ||
import SEOAssessor from "../../src/scoring/assessors/seoAssessor"; | ||
import contentAssessor from "../../src/scoring/assessors/contentAssessor"; |
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.
Capital C as it's a class?
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.
Do we link to this file from somewhere in the code, or perhaps on Confluence? Could you check?
@@ -6,27 +6,27 @@ import { forEach, has, includes, isEmpty, isNull, isObject, isString, isUndefine | |||
import { getLogger } from "loglevel"; | |||
|
|||
// YoastSEO.js dependencies. |
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.
This comment is a little bit outdated 😅 Could you change the order of the imports so all assessors are together? Thanks!
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.
Git didn't quite understand the move of some files. I think the new code is correct but could you double-check yourself as well? Thanks!
Co-authored-by: Martijn van der Klis <martijnvanderklis@yoast.com>
…e-all-assessors-in-indexjs
…e-all-assessors-in-indexjs
Context
yoastseo
package, we only expose the following assessors: Assessor, ContentAssessor, SeoAssessor, and TaxonomyAssessor. The other assessors such as the related keyword and the e-commerce assessors are not exposed.yoastseo
package can retrieve those assessors fromindex.js
, instead of using complete path.yoastseo
package can easily retrieve the language helpers and configs without the need to import the individual language files.Summary
This PR can be summarized in the following changelog entry:
yoastseo
modules insidecontent-analysis-api
app.yoastseo
modules insidecontent-analysis
app.Relevant technical choices:
Test instructions
Test instructions for the acceptance test before the PR gets merged
This PR can be acceptance tested by following these steps:
To test the exposing of the word complexity helpers
To test the exposing of the assessors
Related Keyphrase
Product assessors
Content analysis API app
Relevant test scenarios
Test instructions for QA when the code is in the RC
QA can test this PR by following these steps:
Impact check
This PR affects the following parts of the plugin, which may require extra testing:
UI changes
Other environments
[shopify-seo]
, added test instructions for Shopify and attached theShopify
label to this PR.Documentation
Quality assurance
Innovation
innovation
label.Fixes #