Repository navigation
fix: trick build into including dependencies - #27858
Conversation
|
Pinging @elastic/kibana-canvas |
💚 Build Succeeded |
mistic
left a comment
There was a problem hiding this comment.
@w33ble this LGTM as a quick fix for the problem we're having right now. Can we just fix the description inside the server/build_fix.js file?
For the long term, do you think we can make the canvas_plugin_src able to be statically analysed ? Maybe we can create an index file for canvas_plugin_src and import at least one time from there on canvas?
There was a problem hiding this comment.
Just to note that Kibana build is already statically analyzing each x-pack plugin starting from their index files and looking for the dependencies in every files imported from those plugins starting from the index files.
The problem here is that canvas_plugin_src will be built with the webpack and canvas files are importing canvas_plugin and not canvas_plugin_src. Can we only fix the description here @w33ble ? 😃
There was a problem hiding this comment.
@mistic Sure. I think you mentioned previously that we could update the build to look in other paths to resolve the dependencies for the build, would that be hard to add? Basically, everything under canvas_plugin_src besides functions/browser needs to end up in the build.
There was a problem hiding this comment.
Do you think we could create an index file for the canvas_plugin_src server code and import from it at least one time on canvas @w33ble ? If we achieve that I think we'll have a better solution as we will use the same method to search across every x-pack plugin
There was a problem hiding this comment.
I can do that. Seems like a better quick fix anyway. It'll make the server bundle larger than it needs to be I think, but that doesn't seem like a big deal.
There was a problem hiding this comment.
Humm, we are not bundling server side code right? And maybe we can create the index only for the common and server side code like we are doing for the other xpack plugins. What do you think @w33ble 😃 ?
EDIT: I just saw you already made the changes 🎉
import server and common functions so the build correctly includes their dependencies
b104e40 to
6fd3bb7
Compare
|
Can confirm that the most recent changes do still fix the build. @mistic Mind giving me an LGTM comment if this looks good? It's changed since you originally approved it, and I'm guessing based on this comment you like it, but confirmation would be good. |
💚 Build Succeeded |
import server and common functions so the build correctly includes their dependencies
Closes #27729
Tricks webpack into including dependencies it wouldn't normally pick up from
canvas_plugin_src. I tested this, and it fixes the issue with thepointseriesfunction in the build.This is the simplest (and ugliest) fix for the issue. @mistic if you have a better idea for something simple and less ugly, I'd love to hear it.