fix(build): restore exports field in dist/package.json - #105
Conversation
The dist package.json generator used a typo `export` instead of `exports` (reading the non-existent `Existing.export`), so the published package lost its `exports` map. Without it, bundlers resolve the CJS build and the `@effector/reflect/scope` subpath breaks; the CJS entry also triggers a dual-package hazard with effector-react, losing the Provider scope inside `variant`/`reflect`. Also add a guard in validate_dist.mjs so a missing exports field fails the dist validation instead of being silently ignored. Closes effector#104
|
|
kireevmp
left a comment
There was a problem hiding this comment.
Hey there! Thanks for taking the time to open this.
I think this is good to go either way as a fix, but would appreciate if you can take a look at a comment I left below.
| try { | ||
| const publintResult = await $`pnpm publint ${pkgDir}`; | ||
| } catch (error) {} |
There was a problem hiding this comment.
I believe publint can actually do the work of checking exports for us. This generates a "suggestion" but is plenty informative to us.
pkg.moduleis used to output ESM, butpkg.exportsis not defined. As NodeJS doesn't readpkg.module, the ESM output may be skipped. Consider addingpkg.exportsto export the ESM output.pkg.modulecan usually be removed alongside too.
Would you be open to swapping the zx call for publint's Node API? Something similar to
const result = await publint({ pkgDir, strict: true }); // import from 'publint'
if (result.messages.length > 0) throw new Error(/* message */)This would removing manual "exports" check in favor of a supported tool we already use, but didn't gate on.
Problem
build.mjsgeneratesdist/package.jsonfrom a whitelist of fields, but uses a typoexport: Existing.export(singular, non-existent) instead ofexports: Existing.exports(introduced in cef40e6). As a result the published 10.1.0 package has noexportsfield.Impact
@effector/reflect/scopesubpath no longer resolves.main(CJS). The CJS entryrequires its own copy ofeffector-react, causing a dual-package hazard:<Provider>(ESM) anduseUnitinsidevariant/reflect(CJS) reference different effector-react instances, so the forked scope is lost —variantreads the store default instead of the scoped value.Reproduced in a React 19 app:
variant({ if: $store, then, else })under<Provider value={fork(...)}>rendered thethenbranch even though the scoped value wasfalse. Pinning back to 10.0.2 (which still shippedexports) fixed it.Fix
build.mjs:export→exports.validate_dist.mjs: fail when the generateddist/package.jsonis missing theexportsfield (././scope). The existingattw/publintcalls swallow their errors, so the regression went unnoticed.Verify
pnpm buildnow emitsdist/package.jsonwith the fullexportsmap (both.and./scopeconditions).Closes #104