-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Expressions refactor #54342
Expressions refactor #54342
Conversation
Pinging @elastic/kibana-app-arch (Team:AppArch) |
💔 Build FailedTest FailuresKibana Pipeline / kibana-xpack-agent / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/lens/smokescreen·ts.lens app lens smokescreen tests should allow seamless transition to and from table viewStandard Out
Stack Trace
Kibana Pipeline / kibana-xpack-agent / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/lens/smokescreen·ts.lens app lens smokescreen tests should allow seamless transition to and from table viewStandard Out
Stack Trace
History
To update your PR or re-run it, just comment with: |
💔 Build FailedTest FailuresKibana Pipeline / kibana-xpack-agent / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/lens/smokescreen·ts.lens app lens smokescreen tests should allow seamless transition to and from table viewStandard Out
Stack Trace
Kibana Pipeline / kibana-xpack-agent / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/lens/smokescreen·ts.lens app lens smokescreen tests should allow seamless transition to and from table viewStandard Out
Stack Trace
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Canvas changes look good, but one little issue with some of our types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall code changes LGTM! I skimmed the Canvas & Lens updates but did a closer read on the internal Expressions plugin code.
In particular I think the naming changes ("input", "output", and "execution context"), while a bit of a pain to implement now, will make the whole system more intuitive in the long run, so thanks for doing that.
Adding a few nits to consider, but nothing urgent enough that it needs to block merging IMO.
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
* master: (27 commits) Include actions new platform plugin for codeowners (elastic#57252) [APM][docs] 7.6 documentation updates (elastic#57124) Expressions refactor (elastic#54342) [ML] New Platform server shim: update annotation routes to use new platform router (elastic#57067) Remove injected ui app vars from Canvas (elastic#56190) update max_anomaly_score route schema to handle possible undefined values (elastic#57339) [Add panel flyout] Moving create new to the top of SavedObjectFinder (elastic#56428) Add mock of a legacy ui api to re-enable Canvas storybook (elastic#56673) [monitoring] Removes noisy event received log (elastic#57275) Remove use of copied MANAGEMENT_BREADCRUMBS and use `setBreadcrumbs` from management section's mount (elastic#57324) Advanced Settings management app to kibana platform plugin (elastic#56931) [ML] New Platform server shim: update recognize modules routes to use new platform router (elastic#57206) [ML] Fix overall stats for saved search on the Data Visualizer page (elastic#57312) [ML] [NP] Removing ui imports (elastic#56358) [SIEM] Fixes failing Cypress tests (elastic#57202) Create observability CODEOWNERS reference (elastic#57109) fix results service schema (elastic#57217) don't register a wrapper if browser side function exists. (elastic#57196) Ui Actions explorer example (elastic#57006) Fix update alert API to still work when AAD is out of sync (elastic#57039) ...
* Expressions refactor (#54342) * feat: 🎸 add UiComponent interface * feat: 🎸 add adapter for react => ui components and back * refactor: 💡 move registries to shared /common folder * feat: 🎸 create expressions service state contaienr * chore: 🤖 export some symbols * feat: 🎸 add Executor class * test: 💍 add simple integration test * feat: 🎸 move registries into executor * feat: 🎸 add initial implementation of Execution * feat: 🎸 move Executor's state container into a signle file * refactor: 💡 move createError() to /common folder * feat: 🎸 use Executor in plugin definition * refactor: 💡 rename handlers to FunctionHandlers * feat: 🎸 improve function typings * feat: 🎸 move types and func in sep folder, improve Execution * refactor: 💡 cleanup expression_types folder * refactor: 💡 improve typing names of expression types * refactor: 💡 remove lodash from ExpressionType and improve types * test: 💍 add ExpressionType tests * refactor: 💡 remove function wrappers around types * refactor: 💡 move functions to /common * test: 💍 improve expression function tests * feat: 🎸 create /parser folder * refactor: 💡 move function types into /expression_functions dir * refactor: 💡 improve parser setup * refactor: 💡 fix export structure and move args into expr_func * test: 💍 add ExpressionFunctionParameter tests * fix: 🐛 fix executor types and imports * refactor: 💡 move getByAlias to plugin, fix Execution types * feat: 🎸 add function for argument parsing * test: 💍 add Executor type tests * test: 💍 add executor function and context tests * test: 💍 check that Executor returns Execution * test: 💍 add basic tests for Execution * test: 💍 add basic test for execution of a chain of functions * test: 💍 add "mult" function tot tests * feat: 🎸 create separate expression_renderer folder * feat: 🎸 use new executor in public plugin * feat: 🎸 remove renderers from executor, add result to execution * fix: 🐛 fix Kibana TypeScript errors * test: 💍 add file to write integration tests for expr plugin * refactor: 💡 move state_containers to /common * refactor: 💡 move /parser to /ast and inline format() function * refactor: 💡 remove remaining @kbn/interpreter imports * feat: 🎸 better handling and typing for Executor context * feat: 🎸 use Executor.run function in plugin * fix: 🐛 fix TypeScript type errors * test: 💍 move integration tests into one file * feat: 🎸 create ExpressionsService * chore: 🤖 clean up legacy code * feat: 🎸 use ExpressionsService in /public * refactor: 💡 move inspector adapters to /common * feat: 🎸 improve execution * feat: 🎸 add state to execution state and don't clone AST * test: 💍 add tests for Execution object * test: 💍 improve expression test helpers * test: 💍 add Execution tests * refactor: 💡 improve required argument checking * fix: 🐛 fix Kibana TypeScript errors * test: 💍 add ExpressionsService unit tests * fix: 🐛 fix Expression plugin TypeScript types * refactor: 💡 prefix React component with React* * fix: 🐛 fix X-Pack TypeScript errors * fix: 🐛 fix test TypeScript errors * fix: 🐛 fix issues preventing loading * feat: 🎸 remove getInitialContext() handler * fix: 🐛 fix TypeScript errors * chore: 🤖 remove uicomponent interface * chore: 🤖 remove missing import * fix: 🐛 correctly handle .query in "kibana" expression function * refactor: 💡 call first arg in expression functions "input" * fix: 🐛 do not free Execution state container * test: 💍 fix tests after refactor * test: 💍 fix more tests after refactor * fix: 🐛 remove redundant export * test: 💍 update intepreter_functional test shapshots * fix: 🐛 relax "kibana" function throwing on missin gsearch ctx * refactor: 💡 don't use ExpressionAST interface in Canvas * docs: ✏️ improve ExpressionRenderer JSDocs * refactor: 💡 rename context.types to inputTypes in internal fn * refactor: 💡 replace context.types by unknown in ExprFuncDef * refactor: 💡 improve expression function definitions in OSS * fix: 🐛 correctly set name on metric_vis_fn * refactor: 💡 improve Lens definitions of expression functions * refactor: 💡 improve Canvas expression function definitions * test: 💍 add createMockExecutionContext() helper * refactor: 💡 add some type to events$ observable in expr handler * feat: 🎸 add types to observables in data handler * refactor: 💡 use inputTypes in canvas * fix: 🐛 fix interpreter grammer generation script * feat: 🎸 allow array in getByAlias * test: 💍 simplify test function specs * test: 💍 fix autocomplete tests * fix: 🐛 use correct expression types and NP getFunctions() API * refactor: 💡 use NP expressions to get renderer * fix: 🐛 use context.types on server-side Canvas function defs * refactor: 💡 use NP API to register Canvas renderers * feat: 🎸 use NP API to get types * style: 💄 minor formatting changes * feat: 🎸 use NP API to get expression functions * fix: 🐛 fix Canvas workpads * test: 💍 add missing mock functions * refactor: 💡 improve Lens func definition argument types * fix: 🐛 fix Lens type error * feat: 🎸 make lens datatable work again * feat: 🎸 bootstrap ExpressionsService on server-side * feat: 🎸 expose more registry related functions in contract * feat: 🎸 add environment: server to server-side expressions * docs: ✏️ add documentation * test: 💍 add missing Jest mocks * fix: 🐛 correct TypeScript type * docs: ✏️ improve documentation * fix: 🐛 make FunctionHelpDict type contain only Canvas functions * fix: 🐛 fix merge conflict * test: 💍 fix expression mocks * fix: fix TypeScript disabled help type Co-authored-by: Elastic Machine <[email protected]>
Summary
Closes #44196
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.This was checked for cross-browser compatibility, including a check against IE11Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n supportThis was checked for keyboard-only and screenreader accessibilityFor maintainers
Dev Docs
...
context.types
👉inputTypes