Skip to content

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Oct 13, 2019

Please don't merge this yet

Testing:

  • if codesandbox CI runs

  • how it works with transitive dependencies (the codesandbox from kent only has a direct dependency on @testing-library/react but we would like to resolve its dependency on @testing-library/dom to the canary from this PR as well)

    Edit: It doesn't at the moment so we use https://github.com/kentcdodds/dom-testing-library-with-anything

@eps1lon
Copy link
Member Author

eps1lon commented Oct 13, 2019

It might be that either only mui-org is whitelisted or it doesn't pick up that @testing-library/dom is a transitive dependency.

@CompuIves Could you take a look or forward this to someone who's working on codesandbox CI?

@alexkrolick
Copy link
Collaborator

alexkrolick commented Oct 13, 2019

I assume it will only run once it is on master (?)

@eps1lon
Copy link
Member Author

eps1lon commented Oct 14, 2019

I assume it will only run once it is on master (?)

It will run on PRs.

I forgot the e-mail back though. Repositories need to be explicitly whitelisted.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 15, 2019

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 8737c63:

Sandbox Source
react-testing-library-examples Configuration

@CompuIves
Copy link

CompuIves commented Oct 15, 2019

This is a really interesting case! So it would make sense if we add an extra option, something like:

"dependencyResolutions": {
  "@testing-library/react": "canary"
}

so the sandbox adds this dependency as this version?

@eps1lon
Copy link
Member Author

eps1lon commented Oct 15, 2019

@CompuIves Yes. Though you probably meant "@testing-library/dom": "canary". I'd rather see codesandbox implement the resolutions field which improves interop with yarn.

Or did you mean we need to specify each dependency individually where we want codesandbox to resolve indirect dependencies to the canary?

@CompuIves
Copy link

Or did you mean we need to specify each dependency individually where we want codesandbox to resolve indirect dependencies to the canary?

Right, with my example it would additionally install @testing-library/react@canary together with the built version of @testing-library/dom. I could also implement the resolution field in the bundler if that's better, but it would take longer.

@eps1lon
Copy link
Member Author

eps1lon commented Oct 16, 2019

Right, with my example it would additionally install @testing-library/react@canary

There wouldn't be one. And even if you would need to build it so that the dependencies entry for @testing-library/dom points to the canary.

@CompuIves
Copy link

Hmm, I'm not sure if we're talking about the same thing. Is this to make testing-library/dom-testing-library work with CodeSandbox CI so you can test it for PRs? And with canary you mean the canary tag on npm or the latest build from CodeSandbox CI?

@eps1lon
Copy link
Member Author

eps1lon commented Oct 16, 2019

/react is depending on /dom. I want codesandbox to resolve /dom to the built version of codesandbox CI. Right now we have two versions when building /dom in codesandbox: the release on npm and the release on codesandboc

@eps1lon eps1lon force-pushed the test/codesandbox-ci branch from 5603c87 to 5d5374b Compare October 30, 2019 07:57
@kentcdodds
Copy link
Member

Let me know when you're ready for me to merge this.

@eps1lon
Copy link
Member Author

eps1lon commented Oct 31, 2019

Let me know when you're ready for me to merge this.

@kentcdodds I'm using react-testing-library-examples since I couldn't find another working codesandbox that uses @testing-library/dom. kentcdodds/dom-testing-library-with-anything isn't working in codesandbox.

@eps1lon
Copy link
Member Author

eps1lon commented Oct 31, 2019

This isn't useful at the moment since transitive dependencies of packages on npm are not resolved to the version built in CodeSandbox CI even with resolutions: https://codesandbox.io/s/jolly-ellis-3wjgm

@kentcdodds
Copy link
Member

I'm using react-testing-library-examples

That's fine

@eps1lon
Copy link
Member Author

eps1lon commented Nov 1, 2019

Now it's working again 😄 Let's merge this and see if it happens again.

@eps1lon eps1lon requested a review from kentcdodds November 1, 2019 07:09
@kentcdodds kentcdodds merged commit d5d4e39 into testing-library:master Nov 2, 2019
@kentcdodds
Copy link
Member

Thanks!

@kentcdodds
Copy link
Member

@all-contributors please add @eps1lon for infrastructure

@allcontributors
Copy link
Contributor

@kentcdodds

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

@eps1lon eps1lon deleted the test/codesandbox-ci branch November 2, 2019 07:55
@kentcdodds
Copy link
Member

🎉 This PR is included in version 6.10.1 🎉

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.

4 participants