[iOS][swiftpm] Documentation updates - #58084
Conversation
meta-cla
Bot
added
the
CLA Signed
|
Caution Invalid Base Branch The base branch for this PR is |
The derivation rule, the `spm.name` override and the names React Native reserves were only discoverable from the autolinker's source or from the error a collision produces. Write them down where a library author looks: the scoped package case that surprises people (`@scope/react-native` derives `ReactNative`, and gets its scope back as `ScopeReactNative`), the two reasons to set `spm.name` and its charset, the reserved names in one table, what happens when two libraries derive the same name — who moves, who stays, and when the build fails anyway — and the npm `files` requirement that makes the override reach consumers at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
cdaf50b to
dfdb704
Compare
a859dcc to
0d7aeb7
Compare
cipolleschi
left a comment
cipolleschi
left a comment
There was a problem hiding this comment.
thanks for putting these docs together. I left a few suggestions and questions.
| - **One target.** `spm add` attaches the `Autolinked` aggregate to a single app | ||
| target (`--productName` picks it when the project has several), and modules | ||
| reach only that target. Known gap: rn-tester's test targets | ||
| (`RNTesterUnitTests`, `RNTesterIntegrationTests`) get their own pods under | ||
| CocoaPods, and `spm.modules` has no equivalent for them yet. |
There was a problem hiding this comment.
this is not super clear. it means that an app can only have 1 SPM module? or that all the SPM modules declared by the app are conflated in a single target?
| | `@scope/common` | `Common` | | ||
| | `@scope/react-native` | `ReactNative` | | ||
|
|
||
| The last row is the surprise: dropping the scope makes `@scope/react-native` | ||
| derive `ReactNative`, a [reserved name](#names-react-native-reserves). A scoped |
There was a problem hiding this comment.
can we change the logic so we always keep the scope? It feels easier to implement and to remember, no?
It also helps with clashes.
For example, both Expo and Firebase might be publishing a @expo/common and @firebase/common package.
IF we keep the scoping, we have ExpoCommon and FirebaseCommon, while if we drop it, we have
two Common packages, with a clash.
There was a problem hiding this comment.
This was done to try to always match the existing header files folder name - which seemed to fit better without the scope - meaning less changes for our devs. I also agree that we could just use the scoped name all over - but that might lead to a few more changes by library authors.
There was a problem hiding this comment.
uhm... in that case, it's probably better to fail immediately and asks the user to add a spm.name.
I don't think we have any one that has #include<Common/SomeHeader.h>. This should also simplify the implementation.
There was a problem hiding this comment.
or in general why are we trying to do something smart? I can see more cases where this heuristic will fail than cases where it will work.
For example:
- every
react-native-xxxxprobably does not have#import<ReactNativeXXX/Header>. - every scoped package is unlikely that they will always use the second part of the scope.
Perhaps we should not try to be smart and we should just ask library maintainers to specify that the spm.name has to be their import prefix.
WDYT?
There was a problem hiding this comment.
Great points. What about reading from podspec if it exists and use the header_dir field? Then react-native-svg would default to RNSVG - which is correct? This will be a new change in a separate PR (since the naming stuff is not really changed in this iteration - we just documented and added collision handling (#58044).
Co-authored-by: Riccardo Cipolleschi <cipolleschi@meta.com>
Co-authored-by: Riccardo Cipolleschi <cipolleschi@meta.com>
Co-authored-by: Riccardo Cipolleschi <cipolleschi@meta.com>
The limit read as a cap on how many modules an app may declare, or as all of them being merged into one target. It is neither: each entry is its own SwiftPM target, and the constraint is which targets can use them — only the app target `spm add` attached the aggregate to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The GitHub suggestion commits reflowed prose past the print width, padded a table cell, and left trailing whitespace on a heading — prettier --list-different flags the file, which is what the lint job runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 participants
Summary:
There were some missing explanation regarding naming of SwiftPM modules which led to some issues for library authors, and based on changes in the functionality the docs also needed a few minor updates to support these changes (we're still in preview, so changes are expected).
This PR updates the docs.
Changelog:
[IOS] [FIXED] - Updated SwiftPM documentation
Test Plan:
✅ Verify that the docs are aligned with the implementation