Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Review of module detection (
|
| field | package.json declares | test expects | source that actually won |
|---|---|---|---|
ios minsdk |
5.0.0 |
3.0.0 |
ios manifest |
android minsdk |
7.0.0 |
4.0.0 |
android manifest |
ios apiversion |
2 |
1 |
ios manifest |
android apiversion |
4 |
6 |
android manifest |
ios architectures |
armv7 arm64 i386 x86_64 |
armv7, i386 |
ios manifest |
ios version |
4.3.1 |
2.0.1 |
package.json top level |
android version |
6.0.1 |
2.0.1 |
package.json top level |
The fixture's own name says what it is for. The tests are green because they encode the current behaviour, so this passes CI while the feature does nothing.
Why it matters rather than being cosmetic
Every field the block can carry is genuinely per-platform, and real modules differ. From the registry we generate:
minsdk—ti.mapis Android12.7.0and iOS10.0.0.GA. One value cannot be right for both.architectures— necessarily disjoint (arm64-v8a/armeabi-v7a/x86_64vsarm64/x86_64).version—ti.mapships Android5.7.0and iOS7.3.1under onemoduleid, eighteen months apart.version = pkgJson.version || manifest?.versioncollapses both to the npm package version and never consultspkgTiJson, unlike every neighbouring field.
The precedence is inverted for top-level fields
Where a field is declared at the titanium top level, it beats the platform manifest:
titanium.<prop> → package.json.<prop> → manifest.<prop>
So a single titanium.minsdk overrides both platforms' manifests — the least specific source winning over the most specific. The manifest is the artifact that actually ships inside the module and is per-platform by construction; package.json is one file spanning both.
Suggested resolution order
Most specific first, and the same chain for every field including version:
manifest.<prop> // per-platform, shipped with the artifact
→ titanium.platform[p].<prop> // per-platform, declared by the publisher
→ titanium.<prop> // package-level
→ package.json.<prop> // package-level, weakest
This keeps the manifest authoritative where one exists — which I think matches the intent — while giving titanium.platform real work to do: it becomes the per-platform source for npm packages that ship no manifests at all, which the cross-platform-native-module fixture (platform dirs containing only .gitkeep) shows is a supported shape.
Whichever order you land on, the fixture above is the one to re-assert against, since it is the only one where all four sources disagree.
Minor: name is manufactured from the npm package name
const name = pkgTiJson.name || pkgJson.name.replace(/^@[^/]+\//, '') || manifest?.name;The fixture asserts name: 'cross-platform-npm-package' while moduleid is cross-platform and both manifests say cross-platform-with-manifest-{ios,android} — three different strings for one module. Since name is a build label nothing resolves against (the install directory, tiapp.xml and require() all key on moduleid), this derivation mostly creates a second identifier free to drift from the one that has to be typed correctly. If name is going to be phased out, this is the code path keeping it alive.
The SDK's module templates seed `license: Specify your license`, and nothing prompts anyone to replace it. Three modules under tidev have shipped releases with it still in place — titanium-identity, titanium-onboarding and ti.previewinteraction — and it surfaces anywhere the manifest does. `readManifest()` now warns when it sees one. It deliberately does not throw: the placeholder belongs to a dependency, and failing an app build because somebody else's metadata is unfilled punishes the wrong person. The predicate is exported so a packaging or release step, which is talking to the author who can actually fix it, can reject what this only warns about.
The warning called the value a "scaffolding placeholder", which describes where the string came from rather than what the reader should do about it. It now names the fix: Module ti.foo has not set a license (ios/manifest): choose an SPDX license such as "Apache-2.0". Suggesting SPDX rather than any free-form string is the point: sixteen modules currently spell Apache 2.0 six different ways, which is what made this worth warning about at all.
Warn when a module has not set a license
1 participant
Do NOT merge this PR - for review/CI testing only
We will eventually rename this
v7branch tomain.Titanium CLI integration: tidev/titanium-cli#916
Status:
tiapp.xmlparsertimodule.xmlparserapply()test)PATHoverride