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

Fix re-exporting types from data plugin. #35682

Closed
wants to merge 1 commit into from

Conversation

lukeelmers
Copy link
Member

This resolves an issue originally identified in #35300, where CI would fail due to an attempted import from a .d.ts file. It updates index patterns in the data plugin with a new convention for re-exporting types, which shouldn't cause CI troubles.

Once this is merged it will unblock #35389, #35390, and #35460.

@lukeelmers lukeelmers added :AppArch Feature:Data Views Data Views code and UI - index patterns before 8.0 Feature:New Platform labels Apr 26, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@lukeelmers lukeelmers added release_note:skip Skip the PR/issue when compiling release notes v7.2.0 v8.0.0 labels Apr 26, 2019
}

/** @public types */
export { IndexPattern, StaticIndexPattern, StaticIndexPatternField, Field } from './index_patterns';
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this export pattern to be use only here or for all the sub-components as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

I did it here because the guidance from the Core team was to export all types at the top level (so downstream user's don't need to know about the paths to the subcomponents of your plugin). I suppose we could use a similar pattern at the service-level if we wanted to, the only reason I didn't was because of .d.ts files: it felt cleaner to only re-export types from the service.

In a service class file, if I were to export { Foo } from 'ui/foo'; and Foo is both a type and a module, I'll be exporting both from my service. But if I explicitly import * as types from 'ui/foo'; and export type Foo = types.Foo; it ensures that only the type is being exported.

Related: There was discussion of potentially namespacing types by service in Core (e.g. IndexPatterns.Field), but they haven't done this yet because the current thinking is that, over time, most types people will need to import will already be included in a plugin's Setup interface (DataSetup).

So my current proposed conventions for handling types during migration would be:

  • Export the setup interface from each service (& index.ts), either using ReturnType or by explicitly writing the interface
  • Import everything into the top-level Plugin service
  • Explicitly export the top-level setup interface (DataSetup)
  • Re-export any additional types for services from the top-level

This makes it so that the only place people need to import from is the top-level directory plugins/data

@lukeelmers
Copy link
Member Author

Closing as I believe these changes have already merged in another PR.

@lukeelmers lukeelmers closed this May 20, 2019
@lukeelmers lukeelmers deleted the fix/data-type-imports branch May 20, 2019 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Data Views Data Views code and UI - index patterns before 8.0 Feature:New Platform release_note:skip Skip the PR/issue when compiling release notes v7.2.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants