elastic / search-ui

Search UI. Libraries for the fast development of modern, engaging search experiences.
https://docs.elastic.co/search-ui
Apache License 2.0
1.92k stars 368 forks source link

Import cycles can cause out of memory issues with code processing tools like wyw-in-js #1066

Closed dumbNickname closed 1 month ago

dumbNickname commented 3 months ago

Hi,

I open this issue to check if you would be interested in making react-search-ui package friendly/compatible for processing libraries such as wyw-in-js (https://wyw-in-js.dev/), by removing the dependency cycles. In our case it is linaria (https://github.com/callstack/linaria) that we use to extract the css from react components.

An example of such suspicious import that I found when debugging wyw-in-js is https://github.com/elastic/search-ui/blob/main/packages/react-search-ui/src/index.ts where we find export * from "./containers";

while later in https://github.com/elastic/search-ui/blob/main/packages/react-search-ui/src/containers/ErrorBoundary.tsx we can see import { withSearch } from ".."; which seems to point back to index.js (while withSearch could be directly imported from (https://github.com/elastic/search-ui/blob/main/packages/react-search-ui/src/withSearch.tsx).

The mentioned dependency cycle causes a pretty nasty issue for wyw-in-js, which is an out of memory error. Short summary is - wyw-in-js processes the react-search-ui module, so already has it in cache of "known" modules, but will not set its evaluated flag until module is fully processed with its dependencies.When evaluated flag is missing, an error is thrown and the module is added to the queue of modules to be processed and here the infinite loop happens due to dependency cycle. It took some time to debug and go through memory snapshots to track this down, so I thought that we could try to save some time for other ppl in future, who might stumble upon this.

I could probably make a feature request to wyw-in-js (what might still happen) to support dependency cycles, but I think those are in general considered to be a bad practice so I thought I will try here first :) I just want to know if that import is some very important convention for you or just a mistake that can be fixed.

Thanks in advance for any answer :)

Samiul-TheSoccerFan commented 3 months ago

Thank you for raising this issue with you. we expect Bundlers to handle this better than others.

We would happily accept a PR that works for you on your setup, fixing the component imports that create the circular dependency issue.

botelastic[bot] commented 1 month ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Is this issue still important to you? If so, please leave a comment and let us know. As always, thank you for your contributions.