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-infra] Fix broken sandboxes with relative module imports #42767

Merged
merged 10 commits into from
Aug 9, 2024

Conversation

bharatkashyap
Copy link
Member

@bharatkashyap bharatkashyap commented Jun 26, 2024

@bharatkashyap bharatkashyap added the scope: docs-infra Specific to the docs-infra product label Jun 26, 2024
@mui-bot
Copy link

mui-bot commented Jun 26, 2024

Netlify deploy preview

https://deploy-preview-42767--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against f2ff725

@zannager zannager requested a review from mnajdova June 27, 2024 15:56
@bharatkashyap bharatkashyap requested review from mnajdova and removed request for mnajdova July 5, 2024 20:49
@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work regression A bug, but worse labels Jul 12, 2024
Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>
Signed-off-by: Bharat Kashyap <bharatkashyap@outlook.com>
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 14, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 14, 2024
@alexfauquette
Copy link
Member

@bharatkashyap I created #43145 fro mexperimental purpose.

My concern with the current solution is that you take into consideration that imports are from ./xxx which is not always the case. In MUI X we have two case where we could need deeper imports.

For example the tree-view is looking to define 2 or 3 datasets to reuse every where. So they would likely have path like ../datasets/products.json Same for the charts that might have more data sets but that could be shared across multiple pages (and so multiple folders)

In the PR, I propose to respect the file splitting in the generated codesandbox, but move all the files at the same level.

@bharatkashyap
Copy link
Member Author

[docs-infra][Do not merge] Fix broken sandboxes with relative module imports #43145

Thanks @alexfauquette for this! Your proposal makes sense to me; thanks for bringing to light this oversight :)

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 2, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 8, 2024
@alexfauquette
Copy link
Member

I've replace the package-lock by the current one

The demo seems to work nice on both codesandbox and stackblitz 👍
https://deploy-preview-42767--material-ui.netlify.app/material-ui/react-autocomplete/#combo-box

Copy link
Member

@cherniavskii cherniavskii left a comment

Choose a reason for hiding this comment

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

I tested it with a multi-tab MUI X demo in mui/mui-x#13695 – it works like a charm!
Great job! 🎉

@bharatkashyap bharatkashyap merged commit c18e53d into mui:next Aug 9, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work regression A bug, but worse scope: docs-infra Specific to the docs-infra product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[docs-infra] CodeSandbox/StackBlitz broken for demos with relative imports
7 participants