[File upload][Maps] NP migration for server & client #51045
Conversation
kindsun
added
WIP
|
Pinging @elastic/kibana-gis (Team:Geo) |
kindsun
added
release_note:skip
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
|
retest |
💔 Build Failed |
|
retest |
💚 Build Succeeded |
💚 Build Succeeded |
x-pack/legacy/plugins/file_upload/public/components/json_upload_and_parse.js
Outdated
Show resolved
Hide resolved
💔 Build Failed |
|
retest |
| import { FileUploadPlugin as Plugin } from './plugin'; | ||
|
|
||
| export function plugin() { | ||
| return new Plugin(); |
There was a problem hiding this comment.
no need to alias, just do new FileUploadPlugin
Even beyond that, what is this file adding beyond indirection?
There was a problem hiding this comment.
Yeah, that's about all it's adding here since we're not sharing any static code. I threw it in there to give it the same "shape" as other plugins. Other examples I saw were pretty minimal and didn't add much either. Agreed that aliasing is superfluous here.
| */ | ||
|
|
||
| import { npStart } from 'ui/new_platform'; | ||
| import { plugin } from '.'; |
There was a problem hiding this comment.
Pulled from the example on that one and found a few others who used the same syntax. No strong feelings either way personally so I just stuck with the example syntax
| import { npStart } from 'ui/new_platform'; | ||
| import { plugin } from '.'; | ||
|
|
||
| const pluginInstance = plugin(); |
There was a problem hiding this comment.
Why do we need this module hopping? Is this something from the platform?
import { FileUploadPlugin } from './plugin';
const pluginInstance = new FileUploadPlugin();
There was a problem hiding this comment.
Again just sticking with the expected shape for consistency with other plugins at this stage. Ultimately this file will go away when moved though.
| @@ -6,7 +6,7 @@ | |||
|
|
|||
|
|
|||
| import React from 'react'; | |||
| import { JsonUploadAndParse } from '../../../../../file_upload/public'; | |||
| import { start as fileUpload } from '../../../../../file_upload/public/legacy'; | |||
There was a problem hiding this comment.
see above. if we're going to rename it here, just give it its usable name when exporting in legacy
There was a problem hiding this comment.
I definitely see your point, but I stuck with prevailing conventions on this one. If you do a local code search for "start as" you'll see some NP examples.
|
|
||
| const pluginInstance = plugin(); | ||
|
|
||
| export const start = pluginInstance.start(npStart.core); |
There was a problem hiding this comment.
Why the rename when it is being renamed yet again when it is imported?
There was a problem hiding this comment.
Related to the previous comment on naming it "start". Here's the relevant section in the migration guide.
| const fileType = 'json'; | ||
|
|
||
| export async function indexData(parsedFile, transformDetails, indexName, dataType, appName) { | ||
| if (!parsedFile) { | ||
| throw(i18n.translate('xpack.fileUpload.indexingService.noFileImported', { | ||
| defaultMessage: 'No file imported.' | ||
| })); | ||
| return; |
|
|
||
| public start(core: CoreStart) { | ||
| initServicesAndConstants(core); | ||
| return { |
There was a problem hiding this comment.
any reason this is wrapped in an object?
There was a problem hiding this comment.
Just another convention they used in the guide and I saw used around.
💔 Build Failed |
💔 Build Failed |
|
retest |
💚 Build Succeeded |
|
@elasticmachine merge upstream |
💚 Build Succeeded |
* Use np savedObjectsClient in indexing service * Export File Upload UI via start since it requires initialization * Pass services through top level react component props * Handle basePath ref and 'kbn-version' for requests * Bulk of logic for removing hapi server dependencies for server app * Use request obj subset of original request * Move startup logic over to server plugin file and call from index.js * Update server tests * Clean up * Remove old makeUsageCollector export statement * initServicesAndConstants in the start method instead of in the react component * Review feedback
* Use np savedObjectsClient in indexing service * Export File Upload UI via start since it requires initialization * Pass services through top level react component props * Handle basePath ref and 'kbn-version' for requests * Bulk of logic for removing hapi server dependencies for server app * Use request obj subset of original request * Move startup logic over to server plugin file and call from index.js * Update server tests * Clean up * Remove old makeUsageCollector export statement * initServicesAndConstants in the start method instead of in the react component * Review feedback
This PR reshapes the File Upload plugin to a format compatible with the new platform, i.e.- all required steps up to the point of using shims but not yet moved to the new location. Wherever practical, new services were leveraged. Also includes small updates to the Maps app, which is currently the only app leveraging this plugin.
Server-side
Switch to new platform services(Handle in separate PR pending router work and data and admin cluster handling work)Migrate to the new plugin system(Handle in separate PR)Browser-side
Provide plugin extension points decoupled from angular.js(N/A)Move all webpack alias imports into uiExport entry files(N/A)Migrate to the new plugin system(Handle in separate PR)