chore: move cpp folder into packages/core#404
Conversation
dff8023 to
2da7794
Compare
hryhoriiK97
left a comment
There was a problem hiding this comment.
LGTM! I've tested this locally - I generated a .tgz file and installed it in a newly created RN project - everything works as before. Also, I've left a few comments — please take a look at them 🙏
| s.authors = "Software Mansion" | ||
| s.source = { :git => "https://github.com/software-mansion/react-native-enriched-markdown.git" } | ||
|
|
||
| s.platforms = { :ios => "15.1", :osx => "14.0" } |
There was a problem hiding this comment.
This hardcodes the iOS minimum to 15.1, while ReactNativeEnrichedMarkdown.podspec uses the dynamic min_ios_version_supported from React Native. If the RN minimum changes, these will drift apart and could cause CocoaPods resolution issues. Let's use the same helper here
| s.source_files = "ios/**/*.{h,m,mm,cpp,swift}", "cpp/md4c/*.{c,h}", "cpp/parser/*.{hpp,cpp}" | ||
| else | ||
| s.dependency "EnrichedMarkdownCore" | ||
| end |
There was a problem hiding this comment.
The first two assignments to s.private_header_files and s.source_files are immediately overridden in the unless branch - they only matter for the else (monorepo) path. This reads a bit confusingly. Would be clearer as a straight if/else. What do you think about this?
if monorepo
s.private_header_files = "ios/**/*.h"
s.source_files = "ios/**/*.{h,m,mm,cpp,swift}"
s.dependency "EnrichedMarkdownCore"
else
s.private_header_files = "ios/**/*.h", "cpp/**/*.{h,hpp}"
s.source_files = "ios/**/*.{h,m,mm,cpp,swift}", "cpp/md4c/*.{c,h}", "cpp/parser/*.{hpp,cpp}"
end
| ], | ||
| "scripts": { | ||
| "build:wasm": "bash cpp/wasm/build.sh", | ||
| "build:wasm": "bash ../../packages/core/cpp/wasm/build.sh", |
There was a problem hiding this comment.
Nit: the path goes up to the repo root and back down. Since we're already in packages/react-native-enriched-markdown, this is equivalent and shorter:
"build:wasm": "bash ../core/cpp/wasm/build.sh",
2da7794 to
845a38a
Compare
What/Why?
Moves packages/react-native-enriched-markdown/cpp into packages/core. packages/core will be the source of truth for cpp core and used by other packages.
There is a hack in CmakeLists.txt and ReactNativeEnrichedMarkdown.podspec:
I don't like it but I see no other easy way, because cocoapods doesn't follow symlinks and doesn't allow sources from outside packages/react-native-enriched-markdown. And cmake has issues with following symlinks too. I think the only other solution would be publishing the core separately but we'd rather avoid this?
After react-native-enriched-markdown will depend on the native libraries the release flow will change anyway and probably will be nicer.
Testing
PR Checklist