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

Use api ref #86

Merged
merged 4 commits into from
Jul 20, 2020
Merged

Use api ref #86

merged 4 commits into from
Jul 20, 2020

Conversation

dtassone
Copy link
Member

No description provided.

@dtassone
Copy link
Member Author

This one will not be green as we need to publish the package to fix it

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 20, 2020

This one will not be green as we need to publish the package to fix it

When should we fix this requirement? Maybe we should open an issue about it to keep track of the problem? (It's not OK that we need to publish the component to get the latest update on the documentation preview).

@dtassone
Copy link
Member Author

This one will not be green as we need to publish the package to fix it

When should we fix this requirement? Maybe we should open an issue about it to keep track of the problem? (It's not OK that we need to publish the component to get the latest update on the documentation preview).

Yes we can open an issue. Netlify don't know how to manage monorepos properly yet so if you do a breaking changes on the grid then the storybook and the demo app will be broken until you publish the pkgs

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 20, 2020

Yes we can open an issue. Netlify don't know how to manage monorepos properly yet so if you do a breaking changes on the grid then the storybook and the demo app will be broken until you publish the pkgs

From what I understand, this is the same problem than we have current on requiring to build the components + yarn link in development in order to render them in the storybook. We solve that, we solve Netlify build at the same time.

For the solution, I would suggest we copy & paste the one from the main repo, no need to innovate. I have added a mention to this problem in #50

@dtassone
Copy link
Member Author

Merged and will release

@dtassone dtassone merged commit 93550f9 into master Jul 20, 2020
@dtassone dtassone deleted the useApiRef branch July 20, 2020 16:10
@@ -1,7 +1,7 @@
import * as React from 'react';
import { fromEvent, Subscription } from 'rxjs';
import { takeUntil, tap } from 'rxjs/operators';
import { XGrid, ColDef, GridApi, GridOptionsProp } from '@material-ui/x-grid';
import { XGrid, ColDef, GridOptionsProp, useApiRef } from '@material-ui/x-grid';
Copy link
Member

@oliviertassinari oliviertassinari Jul 20, 2020

Choose a reason for hiding this comment

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

Regarding the name of the modules, I'm pretty confident that they should be prefixed with Grid to avoid all future conflict of modules from @material-ui/x, unless there is almost no chance we have a component with the same name, or type.

import { XGrid, GridColDef, GridOptionsProp, useGridApiRef } from '@material-ui/x-grid';

@oliviertassinari oliviertassinari added the component: data grid This is the name of the generic UI component, not the React module! label Jan 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants