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.

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

feat: add a return value from logTestingPlaygroundURL #1144

Merged

Conversation

Lirlev48
Copy link
Contributor

@Lirlev48 Lirlev48 commented Jul 4, 2022

Adding a return value to logTestingPlaygroundURL function besides the log - Resolves #1125

logTestingPlaygroundURL() used to prints the value directly to the stdout of the running process,
The values was hidden in the log window

Adding a return statement to the logTestingPlaygroundURL function

Checklist:

  • Documentation added to the
    docs site
  • Tests
  • TypeScript definitions updated
  • Ready to be merged

Will add documentation to testing-library-docs after the PR will be completed.

lironlevy added 2 commits July 4, 2022 12:01
@codesandbox-ci
Copy link

codesandbox-ci bot commented Jul 4, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 4bcb210:

Sandbox Source
react-testing-library-examples Configuration

@Lirlev48
Copy link
Contributor Author

Lirlev48 commented Jul 4, 2022

docs for this PR testing-library/testing-library-docs#1105

@codecov
Copy link

codecov bot commented Jul 4, 2022

Codecov Report

Merging #1144 (4bcb210) into main (7b10f73) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main     #1144   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           24        24           
  Lines          987       989    +2     
  Branches       325       325           
=========================================
+ Hits           987       989    +2     
Flag Coverage Δ
node-12 100.00% <100.00%> (ø)
node-14 100.00% <100.00%> (ø)
node-16 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/screen.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7b10f73...4bcb210. Read the comment docs.

types/screen.d.ts Outdated Show resolved Hide resolved
types/screen.d.ts Outdated Show resolved Hide resolved
Lirlev48 and others added 2 commits July 8, 2022 17:03
Co-authored-by: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com>
Co-authored-by: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com>
@Lirlev48
Copy link
Contributor Author

Lirlev48 commented Jul 8, 2022

@timdeschryver can you please approve? thanks

timdeschryver
timdeschryver previously approved these changes Jul 8, 2022
Copy link
Member

@timdeschryver timdeschryver left a comment

Choose a reason for hiding this comment

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

LGTM, leaving it to @MatanBobi

Copy link
Member

@MatanBobi MatanBobi left a comment

Choose a reason for hiding this comment

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

Thanks @Lirlev48 for the contribution :) Left a small comment.

@timdeschryver thanks for reviewing this one :)
Do you see any issue with this? I'm not sure if this change can somehow break something, wdyt?

src/screen.ts Outdated
@@ -42,6 +42,7 @@ const logTestingPlaygroundURL = (element = getDocument().body) => {
console.log(
`Open this URL in your browser\n\n${getPlaygroundUrl(element.innerHTML)}`,
)
return getPlaygroundUrl(element.innerHTML)
Copy link
Member

Choose a reason for hiding this comment

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

Small nit, can we save the `getPlaygroundUrl in a const instead of calling the function twice? Not sure what the performance aspects for it, but better safe than sorry :)

@timdeschryver
Copy link
Member

I think this is safe @MatanBobi .

Copy link
Member

@MatanBobi MatanBobi left a comment

Choose a reason for hiding this comment

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

Thanks @Lirlev48 :)

@MatanBobi MatanBobi merged commit 6d6312f into testing-library:main Jul 11, 2022
@MatanBobi
Copy link
Member

@all-contributors please add @Lirlev48 for code :)

@allcontributors
Copy link
Contributor

@MatanBobi

I've put up a pull request to add @Lirlev48! 🎉

@github-actions
Copy link

🎉 This PR is included in version 8.16.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature] add an option to return value from logTestingPlaygroundURL instead of printing to console
3 participants