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

Remove TS types & interfaces from ES exports #1486

Merged
merged 4 commits into from
Jan 29, 2019

Conversation

chandlerprall
Copy link
Contributor

@chandlerprall chandlerprall commented Jan 28, 2019

Summary

Fixes #1376. This extends our typescript->proptypes babel plugin to also remove any types or interfaces from ExportNamedDeclarations e.g. export { Foo, Bar } & export { Foo, Bar } from './source';. Added this functionality to the existing plugin for two reasons: 1. no build tooling in TS or Babel performs this transformation 2. our plugin already tracks variable type information across imports.

I tested this change in create-react-app by generating an npm module tarball with npm pack and adding the contents of that tarball as a dependency in a new CRA project.

Checklist

- [ ] This was checked in mobile
- [ ] This was checked in IE11
- [ ] This was checked in dark mode
- [ ] Any props added have proper autodocs
- [ ] Documentation examples were added

  • A changelog entry exists and is marked appropriately
  • This was checked for breaking changes and labeled appropriately
  • Jest tests were updated or added to match the most common scenarios
    - [ ] This was checked against keyboard-only and screenreader scenarios
    - [ ] This required updates to Framer X components

@chandlerprall chandlerprall force-pushed the remove-ts-type-exports branch from f141d12 to 2db60f1 Compare January 28, 2019 17:12
}
});
Program: {
enter: function visitProgram(programPath, state) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this enter: {} block is the same as Program was previously, just indented another level. The exit block below is new.

@thompsongl
Copy link
Contributor

Kibana integration checks out: built & linked EUI still passes tests and runs locally

Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

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

Verified export/import resolution via the same CRA integration mentioned in the issue and description for this PR (tarball dep and @elastic/ui import target in js).

@chandlerprall
Copy link
Contributor Author

Did a final check, diffing the contents of es and lib build directories with and without this change. Confirmed that this only affects the types exported from icon and also the rgbDef type exported from color. Fixing the changelog conflict then will merge.

@chandlerprall chandlerprall merged commit 6e102f6 into elastic:master Jan 29, 2019
@chandlerprall chandlerprall deleted the remove-ts-type-exports branch January 29, 2019 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EUI with a CRA standalone project
3 participants