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

Export renderers for renderhtml #32

Merged
merged 4 commits into from
Feb 15, 2024

Conversation

anoukdriessen
Copy link
Contributor

Copy link
Collaborator

@jonathan-waarneming-nl jonathan-waarneming-nl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ziet er prima uit. Ik zou 't nog wel willen zien werken in https://github.com/observation/obsidentify/pull/912, dus als je deze PR in die change kan verwerken, zal ik weer kijken.

Comment on lines +69 to +71
imgRenderer,
olRenderer,
ulRenderer,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waarom is opnieuw exporteren op een andere plek nodig? Kun je in de app niet

import RenderHtmlWrapper, { imgRenderer, olRenderer, ulRenderer } from '@observation.org/react-native-components'

doen?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dan krijg ik: "Module '"@observation.org/react-native-components"' has no exported member 'imgRenderer'." Bij alle renderers waaraan ik export heb toegeveoegd, toen ik ze daarna toevoegde aan de index kreeg ik de melding niet meer, of los ik dat op een andere manier op?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ja, je hebt gelijk. Deze import werkt vermoedelijk wel:

import RenderHtml, { imgRenderer, olRenderer, ulRenderer } from '@observation.org/react-native-components/src/components/RenderHtmlWrapper'

Nadeel is dat we de interne structuur van de component-library gebruiken (src/components). In de huidige setup exporteren we alle componenten als named exports vanaf de root. We verliezen de namespaces die we eerst wel hadden. Alle exports komen in 1 lange lijst van named exports terecht. Wellicht is er een betere oplossing mogelijk. Maar dat doen we niet in deze PR, dus voor nu is het toevoegen van exports in de root van de library wel goed.

@anoukdriessen
Copy link
Contributor Author

Ziet er prima uit. Ik zou 't nog wel willen zien werken in observation/obsidentify#912, dus als je deze PR in die change kan verwerken, zal ik weer kijken.

Ja goeie, heb het toegevoegd!

Copy link
Collaborator

@jonathan-waarneming-nl jonathan-waarneming-nl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@anoukdriessen anoukdriessen force-pushed the feature/export-custom-renderers-renderhtml branch from 5f652b7 to 4e10897 Compare February 15, 2024 11:31
@anoukdriessen anoukdriessen merged commit 9dea356 into develop Feb 15, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants