Conversation
🦋 Changeset detectedLatest commit: 5c2d111 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
fix for dynamic image render
|
Tests do not pass for me with the fix. |
|
Would appreciate this fix a lot! :) |
| * @returns {Array} parsed react elements | ||
| */ | ||
| const createInstances = element => { | ||
| const createInstances = async element => { |
There was a problem hiding this comment.
I don't think this is correct architectural-wise. We are mixing concepts here. createInstances should not fetch images, that's done separately on another layer. Can we preserve that?
There was a problem hiding this comment.
I'm not very familiar with this project and was just looking for a quick solution. Perhaps this issue would be better addressed by someone more acquainted with the codebase.
There was a problem hiding this comment.
I suggested 2 possible approaches here #1369 (comment)
Do they make sense @diegomura?
There are ongoing challenges with dynamically rendering the
component. The primary issue identified was the absence of image properties on the child node. To rectify this, I introduced a new validation step for the Image type, ensuring the creation of a new node instance with the appropriate attributes.
To facilitate this solution, I needed to modify several functions to execute asynchronously, particularly because fetchImage was already an asynchronous function.
As my expertise with react-pdf is limited, I'm hopeful that this solution proves effective. I'm more than open to feedback and help in this area.