Skip to content

Adding first iteration over TSX on new tab #700

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

Merged
merged 4 commits into from
Feb 1, 2021

Conversation

pedroapfilho
Copy link
Contributor

@pedroapfilho pedroapfilho commented Nov 30, 2020

This is the first iteration of the hypothesis of a new tab for tsx code on the examples.

#696

@nickserv
Copy link
Member

It seems like the deploy preview is failing on the Prettier script. Please try committing the results of npm run format-docs.

@pedroapfilho
Copy link
Contributor Author

Hey @nickmccurdy, this is the output:

❯ npm run format-docs

> react-testing-library-docs@ format-docs /Users/pedrofilho/dev/testing-library-docs
> prettier --write '../docs/**/*.mdx'

[error] No matching files. Patterns tried: ../docs/**/*.mdx !**/node_modules/** !./node_modules/** !**/.{git,svn,hg}/** !./.{git,svn,hg}/**
npm ERR! code ELIFECYCLE
npm ERR! errno 2
npm ERR! react-testing-library-docs@ format-docs: `prettier --write '../docs/**/*.mdx'`
npm ERR! Exit status 2
npm ERR!
npm ERR! Failed at the react-testing-library-docs@ format-docs script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/pedrofilho/.npm/_logs/2020-11-30T22_46_52_939Z-debug.log

I have no idea what's going on

@MatanBobi
Copy link
Member

MatanBobi commented Dec 1, 2020

❯ npm run format-docs

> react-testing-library-docs@ format-docs /Users/pedrofilho/dev/testing-library-docs
> prettier --write '../docs/**/*.mdx'

[error] No matching files. Patterns tried: ../docs/**/*.mdx !**/node_modules/** !./node_modules/** !**/.{git,svn,hg}/** !./.{git,svn,hg}/**
npm ERR! code ELIFECYCLE
npm ERR! errno 2
npm ERR! react-testing-library-docs@ format-docs: `prettier --write '../docs/**/*.mdx'`
npm ERR! Exit status 2
npm ERR!
npm ERR! Failed at the react-testing-library-docs@ format-docs script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/pedrofilho/.npm/_logs/2020-11-30T22_46_52_939Z-debug.log

I have no idea what's going on

Looks like I've forgot to change the path for the format-docs script when doing the migration.
I'll fix that in a different PR so you'll be able to run this :)

@MatanBobi MatanBobi mentioned this pull request Dec 1, 2020
@pedroapfilho
Copy link
Contributor Author

Nice @MatanBobi, Thanks!

@alexkrolick
Copy link
Collaborator

@pedroapfilho I merged @MatanBobi 's change - rebase?

@alexkrolick
Copy link
Collaborator

We could install a rebase bot

@pedroapfilho
Copy link
Contributor Author

@MatanBobi I rebased, installed all the packages and ran the npm run format-docs again, and it still fails.

@MatanBobi
Copy link
Member

MatanBobi commented Dec 3, 2020

@MatanBobi I rebased, installed all the packages and ran the npm run format-docs again, and it still fails.

@pedroapfilho you've ran the command locally? I've just cloned your fork and pulled your branch, ran it locally and it succeeded. Is the error you're getting the same as before?

It looks like the CI failed for:

4:26:19 PM: (undefined) TypeError: Cannot read property 'value' of undefined

The same happens when I run npm run build locally so that's probably something there..
I might have time to look into it later on today if you need my help :)

@pedroapfilho
Copy link
Contributor Author

I think that fixing the indentation will fix it

@pedroapfilho
Copy link
Contributor Author

@MatanBobi I think that everything is fine now, can we push it?

@MatanBobi
Copy link
Member

@nickmccurdy since you've commented on the issue, I'd be glad if you'll have a chance to also have a look at this one :)

@MatanBobi
Copy link
Member

@pedroapfilho I'm unable to see the latest commit changes (the ones requested), though I do see them in the commit, any ideas?

@pedroapfilho
Copy link
Contributor Author

As I made the mistake of removing the diff, I just added it back and amended the lest commit

@MatanBobi
Copy link
Member

As I made the mistake of removing the diff, I just added it back and amended the lest commit

Cool, missed that, thanks.
Can you please also rename the js file to jsx as I asked in the comment? :)
After that I think we'll be ready to merge.

@pedroapfilho
Copy link
Contributor Author

I've separated the imports on the 2 tabs, and it's like this now
Screenshot 2021-01-29 at 15 11 00
Screenshot 2021-01-29 at 15 11 03

@MatanBobi
Copy link
Member

This looks great, thanks @pedroapfilho!

@MatanBobi
Copy link
Member

MatanBobi commented Jan 31, 2021

One last thing I saw, sorry for the trouble.
It looks like there's no selected item on first startup since the defaultValue value is js and not jsx could you please fix that and I'll merge? :)
Thanks and sorry again!

@pedroapfilho
Copy link
Contributor Author

Oh! I'm sorry about it, my bad! Fixed. I'll start to put tsx it on other examples now!

@MatanBobi
Copy link
Member

Looks great, thanks @pedroapfilho!
Merging.

@MatanBobi MatanBobi merged commit 50849b0 into testing-library:master Feb 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants