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

[icons] Remove es folder #14518

Merged

Conversation

mgansler
Copy link
Contributor

@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 13, 2019

@mgansler What problem are you trying to solve? (notice that we keep the master branch for important bugs only, on going work is done the next branch)

@oliviertassinari oliviertassinari added the PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI label Feb 13, 2019
@mgansler
Copy link
Contributor Author

@oliviertassinari see related issue #14517

@oliviertassinari
Copy link
Member

@mgansler Thank you! #14517 (comment)


If you don't mind, I would like to repurpose this pull request to reapply #12662. It seems this change was lost: https://packagephobia.now.sh/result?p=%40material-ui%2Ficons%404.0.0-alpha.0.

@mgansler
Copy link
Contributor Author

@oliviertassinari I don't think it got lost, the package.json looks good to me. But I just copied the bits from the core package copy-files script to this one. I removed the line to copy the definitions file to the es/ directory.

@mgansler mgansler force-pushed the fix/14517-createSvgIcon-type-definitions branch 3 times, most recently from b3b0aae to fa0268f Compare February 13, 2019 15:29
@eps1lon
Copy link
Member

eps1lon commented Feb 13, 2019

@mgansler This should be fixed on next. Could you test with @material-ui/icons@next and report back what issue you have with the type declarations?

I'm not even sure we should publish typings for that one. I'm not sure if this is part of the public API.
Well issue #13974 and PR #13994 were accepted so they are indeed public. The publish script just never picked those up.

@oliviertassinari oliviertassinari self-assigned this Feb 14, 2019
@oliviertassinari oliviertassinari force-pushed the fix/14517-createSvgIcon-type-definitions branch from fa0268f to 6521a37 Compare February 14, 2019 10:10
@oliviertassinari oliviertassinari changed the base branch from master to next February 14, 2019 10:11
@oliviertassinari oliviertassinari changed the title Add typescript definition related bits [icons] Add remove es folder Feb 14, 2019
@oliviertassinari oliviertassinari force-pushed the fix/14517-createSvgIcon-type-definitions branch from 61c0f58 to f4e064f Compare February 14, 2019 10:13
@oliviertassinari oliviertassinari added PR: good for merge package: icons Specific to @mui/icons and removed PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI labels Feb 14, 2019
@oliviertassinari oliviertassinari removed their assignment Feb 14, 2019
packages/material-ui-icons/package.json Outdated Show resolved Hide resolved
@eps1lon eps1lon changed the title [icons] Add remove es folder [icons] Remove es folder Feb 14, 2019
@eps1lon eps1lon mentioned this pull request Feb 14, 2019
56 tasks
@oliviertassinari oliviertassinari merged commit a85be10 into mui:next Feb 14, 2019
@mgansler mgansler deleted the fix/14517-createSvgIcon-type-definitions branch February 14, 2019 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: icons Specific to @mui/icons
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants