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

Refactor and simplify expressions plugin in NP #44196

Closed
24 of 28 tasks
streamich opened this issue Aug 27, 2019 · 1 comment · Fixed by #54342
Closed
24 of 28 tasks

Refactor and simplify expressions plugin in NP #44196

streamich opened this issue Aug 27, 2019 · 1 comment · Fixed by #54342
Assignees
Labels
Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline)

Comments

@streamich
Copy link
Contributor

streamich commented Aug 27, 2019

  • Move most of the code into /common folder, so same implementation is shared by browser and server.
  • Remove all imports from @kbn/interpreter.
    • Refactor Registry class
      • Use new Map to store items by ID.
      • Remove .wrapper()
      • Remove .getProp (if possible)
    • Inline what we need to keep from @kbn/interpreter.
      • fromExpression and toExpressions - still importing these but created wrappers and re-exporting those.
      • getType
      • getByAlias
      • castProvider
      • Ast
      • Registry
      • other
    • Remove .toJS() from registries
    • Remove unnecessary functions like initializeInterpreter() and interpretAst(), replace by run() function.
  • Completely refactor interpreterProvider() function
    • Do not mutate AST in irreversible way.
  • Rename expression function context to input.
  • Rename handlers to context.
  • Use state containers
    • State container to store expressions plugin state (registries)
    • New state container for each expression "execution"

Could have:

  • Rename context field in function definition to inputTypes.
  • Instantiate Expression service on server-side, just to see it is working.
  • Add README.
  • Simplify highest level interfaces (loader, renderer).

Follow up work:

  • Move @kbn/interpreter PEG.js build step to expressions plugin.
  • Move @kbn/interpreter toExpression() function to expressions plugin.
  • Remove ExpressionDataHandler in favor of Execution.

Parent issue: #46909

@streamich streamich changed the title [plugins.data.expressions] Remove @kbn/interpreter dependency [Expressions] Remove @kbn/interpreter dependency Aug 27, 2019
@streamich streamich added 3sp Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) labels Aug 27, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch

@streamich streamich changed the title [Expressions] Remove @kbn/interpreter dependency [Expressions] Remove @kbn/interpreter and refactor Aug 30, 2019
@streamich streamich added 8sp and removed 3sp labels Aug 30, 2019
@streamich streamich added 10sp and removed 8sp labels Oct 7, 2019
@streamich streamich mentioned this issue Nov 21, 2019
20 tasks
@streamich streamich removed the 10sp label Nov 21, 2019
@streamich streamich self-assigned this Dec 9, 2019
@streamich streamich changed the title [Expressions] Remove @kbn/interpreter and refactor Remove @kbn/interpreter and refactor Jan 7, 2020
@streamich streamich changed the title Remove @kbn/interpreter and refactor Refactor and simplify expressions plugin in NP Jan 7, 2020
@streamich streamich mentioned this issue Jan 14, 2020
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants