Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(docs): fix typo in packageFilter code example #12851

Merged
merged 1 commit into from
May 16, 2022
Merged

fix(docs): fix typo in packageFilter code example #12851

merged 1 commit into from
May 16, 2022

Conversation

limonte
Copy link
Contributor

@limonte limonte commented May 16, 2022

Summary

The current code example of packageFilter usage is broken and catching the typo isn't trivial without TS.

Test plan

No tests needed.

mrazauskas
mrazauskas reviewed May 16, 2022
View reviewed changes
docs/Configuration.md Outdated
@@ -998,7 +998,7 @@ module.exports = browserResolve.sync;
By combining `defaultResolver` and `packageFilter` we can implement a `package.json` "pre-processor" that allows us to change how the default resolver will resolve modules. For example, imagine we want to use the field `"module"` if it is present, otherwise fallback to `"main"`:

```js
module.exports = (path, options) => {
module.exports = (request, options) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. Docs above mention path, so I would leave it here as is, but would change request below. Just to keep it all in sync.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 👍

@mrazauskas
Copy link
Contributor

Thanks! Please fix this in versioned docs as well. These live at website/versioned_docs. 28.0 and 28.1 are enough in this case.

@limonte
Copy link
Contributor Author

limonte commented May 16, 2022

Thanks! Please fix this in versioned docs as well. These live at website/versioned_docs. 28.0 and 28.1 are enough in this case.

Done 👍

@mrazauskas
Copy link
Contributor

Perfect 👍

SimenB
SimenB approved these changes May 16, 2022
View reviewed changes
Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! can you update the other versions of this doc as well?

@limonte
Copy link
Contributor Author

limonte commented May 16, 2022 •
edited
Loading

thanks! can you update the other versions of this doc as well?

26.x and 27.x are fine, they just consistently use request instead of path. Let me know if you want me to change the agr name to be path across all versions.

@SimenB
Copy link
Member

SimenB commented May 16, 2022

Ah, fair point 👍

@SimenB SimenB merged commit 43d075c into jestjs:main May 16, 2022
This was referenced Jun 13, 2022
Merged
Merged
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mrazauskas mrazauskas mrazauskas left review comments

@SimenB SimenB SimenB approved these changes

Assignees
No one assigned
Labels
cla signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@limonte @mrazauskas @SimenB @facebook-github-bot