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

Typescript sample panel action functional plugin #26747

Conversation

stacey-gammon
Copy link
Contributor

typescripting for functional test plugins

@stacey-gammon stacey-gammon added chore Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Dec 6, 2018
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

@stacey-gammon stacey-gammon force-pushed the 2018-12-05-typescript-sample-panel-action branch from 3a77437 to 07fd3db Compare December 6, 2018 02:55
@stacey-gammon stacey-gammon force-pushed the 2018-12-05-typescript-sample-panel-action branch 2 times, most recently from 3a440fc to 8bf023f Compare December 6, 2018 20:49
@elastic elastic deleted a comment from elasticmachine Dec 6, 2018
@elastic elastic deleted a comment from elasticmachine Dec 6, 2018
@elasticmachine
Copy link
Contributor

💔 Build Failed

@stacey-gammon
Copy link
Contributor Author

hmmm, still failed @spalger, though at a later spot. Screenshot indicates some issues:

screen shot 2018-12-06 at 4 50 28 pm

Looks like jenkins keeps the whole build archive, maybe that will add some illumination. I thought I had set it up at some point to spit out any browser console errors, though I don't see any of those in the jenkins output. 🤔 Will look more next week!

@stacey-gammon
Copy link
Contributor Author

In one of the other failure groups I see:

Cannot redeclare block-scoped variable 'accessibleClickKeys'.
16:06:02 
16:06:02       2   export const accessibleClickKeys: { [keyCode: number]: string };
16:06:02                        ~~~~~~~~~~~~~~~~~~~
16:06:02 
16:06:02         ../../../../node_modules/@elastic/eui/eui.d.ts:2:16
16:06:02           2   export const accessibleClickKeys: { [keyCode: number]: string };
16:06:02                            ~~~~~~~~~~~~~~~~~~~
16:06:02           'accessibleClickKeys' was also declared here.

Maybe I just need to rebase

@stacey-gammon stacey-gammon force-pushed the 2018-12-05-typescript-sample-panel-action branch from 8bf023f to 384b9ac Compare December 8, 2018 19:33
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@stacey-gammon
Copy link
Contributor Author

Same thing, intake fails with:

00:09:42.385 
00:09:42.388 ERROR test/plugin_functional/plugins/kbn_tp_sample_panel_action failed
00:09:42.388       ../../../../node_modules/@elastic/eui/eui.d.ts:1:1 - error TS6200: Definitions of the following identifiers conflict with those in another file: EuiButtonPropsForButtonOrLink, ButtonIconSide, ButtonColor, ButtonSize, EuiButton, ButtonIconColor, EuiButtonIcon, EmptyButtonIconSide, EmptyButtonColor, EmptyButtonSizes, EmptyButtonFlush, EuiButtonEmpty
00:09:42.390 
00:09:42.396       1 declare module '@elastic/eui' {
00:09:42.396         ~~~~~~~
00:09:42.398 
00:09:42.398         node_modules/@elastic/eui/src/components/button/index.d.ts:4:1
00:09:42.399           4 import { SFC, ButtonHTMLAttributes, AnchorHTMLAttributes, MouseEventHandler } from 'react';
00:09:42.399             ~~~~~~
00:09:42.413           Conflicts are in this file.
00:09:42.413 
00:09:42.414       ../../../../node_modules/@elastic/eui/eui.d.ts:1:1 - error TS6200: Definitions of the following identifiers conflict with those in another file: EuiContextMenuPanelHeightChangeHandler, EuiContextMenuPanelTransitionType, EuiContextMenuPanelTransitionDirection, EuiContextMenuPanelShowPanelCallback, EuiContextMenuPanel, EuiContextMenuItemIcon, EuiContextMenuItem, EuiContextMenuPanelId, EuiContextMenuPanelItemDescriptor, EuiContextMenuProps, EuiContextMenu
00:09:42.417 
00:09:42.417       1 declare module '@elastic/eui' {
00:09:42.417         ~~~~~~~
00:09:42.418 
00:09:42.418         node_modules/@elastic/eui/src/components/context_menu/index.d.ts:3:1
00:09:42.418           3 import {
00:09:42.419             ~~~~~~
00:09:42.419           Conflicts are in this file.
00:09:42.420 
00:09:42.420       ../../../../node_modules/@elastic/eui/eui.d.ts:1:1 - error TS6200: Definitions of the following identifiers conflict with those in another file: EuiModal, EuiModalBody, EuiModalFooter, EuiModalHeader, EuiModalHeaderTitle, EUI_MODAL_CONFIRM_BUTTON, EUI_MODAL_CANCEL_BUTTON, EuiConfirmModal
00:09:42.422 
00:09:42.422       1 declare module '@elastic/eui' {
00:09:42.422         ~~~~~~~
00:09:42.422 
00:09:42.423         node_modules/@elastic/eui/src/components/modal/index.d.ts:4:1
00:09:42.424           4 import { ReactNode, SFC, HTMLAttributes } from 'react';
00:09:42.424             ~~~~~~
00:09:42.425           Conflicts are in this file.
00:09:42.426 
00:09:42.426       ../../../../node_modules/@elastic/eui/eui.d.ts:1:1 - error TS6200: Definitions of the following identifiers conflict with those in another file: EuiPage, EuiPageHeader, EuiPageHeaderSection, EuiPageBody, EuiPageContentPaddingSize, EuiPageContentVerticalPosition, EuiPageContentHorizontalPosition, EuiPageContent, EuiPageContentBody, EuiPageContentHeader, EuiPageContentHeaderSection, EuiPageSideBar
00:09:42.428 
00:09:42.429       1 declare module '@elastic/eui' {
00:09:42.429         ~~~~~~~
00:09:42.431 
00:09:42.431         node_modules/@elastic/eui/src/components/page/index.d.ts:4:1
00:09:42.431           4 import { SFC, HTMLAttributes } from 'react';
00:09:42.434             ~~~~~~
00:09:42.434           Conflicts are in this file.
00:09:42.434 

Looks like there are duplicate EUI includes

@stacey-gammon
Copy link
Contributor Author

The error that occurs above during the typeCheck tasks goes away if I remove the added code in projects.ts:

  ...glob
    .sync('test/plugin_functional/plugins/*/tsconfig.json', { cwd: REPO_ROOT })
    .map(path => new Project(resolve(REPO_ROOT, path))),

... I forget why I added that. On a recommendation from @spalger to fix a problem that I now can't remember. I'll remove and see what errors I get. I thought maybe failure to import the ui/public types but it seems okay now (though could also be it's cached and the errors have been recalculated yet in my VSCode).

@elasticmachine
Copy link
Contributor

💔 Build Failed

@stacey-gammon stacey-gammon force-pushed the 2018-12-05-typescript-sample-panel-action branch from e48279f to df63e00 Compare January 15, 2019 20:49
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@spalger
Copy link
Contributor

spalger commented Feb 27, 2019

@mattkime since this is based on #31948 we need to merge that before this, and since we're going to need to rebase for that and a number of the changes in the PR are from that one I'd like to take care of it before reviewing if you don't mind. I believe that we're really close to merging anyway.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@mattkime
Copy link
Contributor

mattkime commented Mar 5, 2019

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💔 Build Failed

@mattkime
Copy link
Contributor

mattkime commented Mar 5, 2019

wow, i think multiple ci sessions were running at once creating a reporting race condition.

@mattkime
Copy link
Contributor

mattkime commented Mar 5, 2019

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@mattkime
Copy link
Contributor

mattkime commented Mar 6, 2019

retest

1 similar comment
@mattkime
Copy link
Contributor

mattkime commented Mar 6, 2019

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@@ -88,8 +89,7 @@
"**/@types/*/**",
"**/grunt-*",
"**/grunt-*/**",
"x-pack/typescript",
"kbn_tp_*/**"
"x-pack/typescript"
Copy link
Contributor

@mattkime mattkime Mar 6, 2019

Choose a reason for hiding this comment

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

@mistic Wanted to get your opinion on this change. Typescript was giving @spalger and I some trouble and this seemed necessary to resolve it. Is there anything we might have inadvertently broke?

(sorry for the ping @ThiagoTcorsi)

Copy link
Member

Choose a reason for hiding this comment

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

@mattkime as far as I remembered, in the past we had the need to add "kbn_tp_*/**" to the yarn workspaces nohoist feature in order to have the node_modules installed for the kibana/test/plugin_functional/plugins, otherwise those plugins during the tests can't read the node_modules they declare from the dll bundle

Copy link
Contributor

Choose a reason for hiding this comment

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

@mistic so if it passes, its good?

Copy link
Member

Choose a reason for hiding this comment

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

@mattkime well maybe I'm missing something here, but what have changed in order to avoid the dependencies for that plugins being hoisting by the yarn workspaces?

@mattkime
Copy link
Contributor

mattkime commented Mar 6, 2019

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@spalger spalger removed their request for review March 6, 2019 21:24
@spalger
Copy link
Contributor

spalger commented Mar 6, 2019

Removed my request for review for the time being, let me know if you want me to take a look at something specific before CI is green

@mattkime
Copy link
Contributor

mattkime commented Mar 7, 2019

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@mattkime mattkime requested a review from spalger March 7, 2019 16:24
@elasticmachine
Copy link
Contributor

💔 Build Failed

@stacey-gammon
Copy link
Contributor Author

Closing this in favor of https://github.com/elastic/kibana/pull/33602/files where I hopefully cherrypicked the necessary changes to get this to build. I think I messed up the merge with master, and there are so many commits it's hard to follow exactly what was necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants