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

[docs] Replace latests tags with next in the codesandbox #21851

Merged
merged 5 commits into from Jul 21, 2020

Conversation

mnajdova
Copy link
Member

@mnajdova mnajdova added the docs Improvements or additions to the documentation label Jul 20, 2020
@mui-pr-bot
Copy link

mui-pr-bot commented Jul 20, 2020

Details of bundle changes

Generated by 🚫 dangerJS against b8f6d4e

@oliviertassinari oliviertassinari changed the title [Docs] Replaced latests tags with next in the codensadbox examples [docs] Replace latests tags with next in the codesandbox Jul 20, 2020
Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

@oliviertassinari
Copy link
Member

@eps1lon On a broader note, we have discussed with Marija the option to use the codesandbox-ci version of the dependencies when building the preview of the pull request.

@eps1lon
Copy link
Member

eps1lon commented Jul 20, 2020

On a broader note, we have discussed with Marija the option to use the codesandbox-ci version of the dependencies when building the preview of the pull request.

You mean using resolved versions e.g. 16.13.1 or 4.9.3 instead of npm distribution tags e.g. latest or next? Had this on my mind for quite some time but never looked at the implementation. Definitely agree with the problem description. Distribution tags don't work well for codesandboxes. Especially if they're used for issue reports.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 20, 2020

You mean using resolved versions e.g. 16.13.1 or 4.9.3 instead of npm distribution tags e.g. latest or next?

I was wondering about something is this order to allow testing the new demos in isolation from the Netlify preview of the pull requests:

diff --git a/docs/src/modules/utils/helpers.js b/docs/src/modules/utils/helpers.js
index c4ef4d8af5..29582b0fc9 100644
--- a/docs/src/modules/utils/helpers.js
+++ b/docs/src/modules/utils/helpers.js
@@ -98,7 +98,7 @@ function getDependencies(raw, options = {}) {
   const versions = {
     'react-dom': reactVersion,
     react: reactVersion,
-    '@material-ui/core': 'latest',
+    '@material-ui/core': `https://pkg.csb.dev/mui-org/material-ui/commit/${process.env.COMMIT_REF}/@material-ui/core`,
     '@material-ui/icons': 'latest',
     '@material-ui/lab': 'latest',
     '@material-ui/styles': 'latest',

@eps1lon
Copy link
Member

eps1lon commented Jul 20, 2020

Ah ok, makes sense. This is a separate issue though. This change affects actual deployed docs versions.

@mnajdova
Copy link
Member Author

Should we have something like this by default:
'@material-ui/core': 'https://pkg.csb.dev/mui-org/material-ui/commit/${process.env.COMMIT_REF}/@material-ui/core',
but use the next/latest for the prod build?

@eps1lon
Copy link
Member

eps1lon commented Jul 20, 2020

I'm not fully convinced this is a good idea. You want your local or prod environment to be as close as possible to the one you actually deploy. This idea completely changes how the environments behave. Do you create a csb that often from a deploy preview and want to test the version deployed to csb? How much time does it take you currently?

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 20, 2020

On a side note. I think that the change that will yield the most value is the one you are working on (latest -> next for the users).

Regarding using codesandbox-ci, it was meant as a suggestion to help the contributors. It started with Marija question around trying the utility classes out, which also resonate with people that want to try the changes our when reviewing pull requests.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 20, 2020

Do you create a csb that often from a deploy preview and want to test the version deployed to csb? How much time does it take you currently?

@eps1lon I do once a week, at most. It takes about 5 minutes to open codesandbox CI. It's far from obvious that this exist for others, hard to find.

@mnajdova
Copy link
Member Author

So should we just fix the tests on this PR and leave the ‘next’ tag?

mnajdova and others added 2 commits July 21, 2020 08:59
Co-authored-by: Matt <github@nospam.33m.co>
@mnajdova mnajdova merged commit f499e9e into mui:next Jul 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants