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] Clarify on how to use the local distribution in the CONTRIBUTING file #13312

Merged
merged 1 commit into from
Oct 20, 2018
Merged

[docs] Clarify on how to use the local distribution in the CONTRIBUTING file #13312

merged 1 commit into from
Oct 20, 2018

Conversation

nicolasiensen
Copy link
Contributor

In this commit, we are clarifying how to install the local distribution of Material-UI to test changes in a real case scenario.

It turns out that, if you don't point the installation to the /build folder when importing a Material-UI component, the path @material-ui/core/<ComponentName> does not work.

It took me a while to realize how to install the local distribution, that's why I decided to update the documentation.

In this commit, we are clarifying how to install the local distribution 
of Material-UI to test changes on a real case scenario.

It turns out that, if you don't point the instalation to the /build 
folder, when importing a Material-UI component, the path 
"@material-ui/core/<ComponentName>" does not work.

It took me a while to realize how to install the local distribution, 
that's why I decided to update the documentation.
@oliviertassinari oliviertassinari changed the title [Documentation] Clarify on how to use the local distribution in the CONTRIBUTING file [docs] Clarify on how to use the local distribution in the CONTRIBUTING file Oct 20, 2018
@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation label Oct 20, 2018
@oliviertassinari oliviertassinari merged commit c26a0c8 into mui:master Oct 20, 2018
@oliviertassinari
Copy link
Member

Thank you.

@nicolasiensen nicolasiensen deleted the clarify-using-local-distrib branch October 20, 2018 13:56
@eps1lon
Copy link
Member

eps1lon commented Oct 20, 2018

For testing with a local distribution we should probably recommend linking (yarn link) instead of file:*. If one rebuilds the package then file:* will still point to the old build. In addition we can explore how we provided incremental builds (i.e. if I change the Button I don't expect Dialog to be rebuild too) of our packages.

At least that's how [email protected] works for me. @nicolasiensen is file:* pointing to the new build if you run yarn build in the material-ui package or do you have to run yarn again in your package?

@nicolasiensen
Copy link
Contributor Author

nicolasiensen commented Oct 20, 2018

Indeed, just after submitting this PR I realized we could use yarn link instead to test the local distribution.

Using file:*, every time you make a change in the library you have to rebuild it and run yarn install from the project you are using to test the changes, it's a very time-consuming process that slows down the feedback loop a lot.

I'm going to try to set up my local environment with yarn link and submit a new change to the documentation to suggest a better approach to test the local distribution.

@eps1lon
Copy link
Member

eps1lon commented Oct 20, 2018

@nicolasiensen I know you figured it already out with file but be sure to run yarn link inside material-ui/build not just in material-ui/

@nicolasiensen
Copy link
Contributor Author

I managed to make it work with yarn link, so whenever I have a change in packages/material-ui/build the app that is using my local distribution automatically reloads itself to apply the changes (I'm using create-react-app to create the test app).

Nevertheless, I still have to manually rebuild the library when a change is introduced, and the build process is going to process all the components, even if I'm changing just one, taking more than 25s.

It would be cool if yarn could watch the changes I'm doing in packages/material-ui/src, and rebuild just the changed components. Do you think this is feasible?

@eps1lon
Copy link
Member

eps1lon commented Oct 22, 2018

Have you tried adding --watch? Depending on your module type either yarn build:es --watch or yarn build:es2015 --watch. Not sure how clever babels watchmode is.

You could also just use babel-plugin-module-resolver and point to the source directly from your code. We use this for our docs-development environment: https://github.com/mui-org/material-ui/blob/1023e9676cf20068bd1d744b7a49a5482d22921d/babel.config.js#L72

Overall I think you shouldn't be doing that. If you work on a fix for this package and want manual e2e testing use yarn docs:dev and add an example in our docs. This offers better isolation and potential overhead when you file a PR upstream.

eps1lon pushed a commit that referenced this pull request Oct 23, 2018
* Recommend yarn link to test local distribution

In this commit, we are updating the CONTRIBUTING file to recommend using
"yarn link" to test the local distribution of Material-UI.

This approach was discussed in #13312

* Empty commit to force codecov to work
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.

3 participants