Skip to content

chore: add support for both cjs and esm builds#41

Open
bleadof wants to merge 1 commit intofinal-form:masterfrom
asteroidcrew:fix/add-cjs-and-esm
Open

chore: add support for both cjs and esm builds#41
bleadof wants to merge 1 commit intofinal-form:masterfrom
asteroidcrew:fix/add-cjs-and-esm

Conversation

@bleadof
Copy link
Copy Markdown

@bleadof bleadof commented Dec 2, 2025

  • add .node-version file
  • use cjs for nps script
  • add the installed plugins to the eslint config
  • add both esm and cjs builds
    • My next.js project that still uses babel doesn't build without this

@bleadof bleadof marked this pull request as draft December 2, 2025 18:24
- add .node-version file
- use cjs for nps script
- add the installed plugins to the eslint config
- add both esm and cjs builds
  - My next.js project that still uses babel doesn't build without this
@bleadof bleadof force-pushed the fix/add-cjs-and-esm branch from fbab5b3 to 19c20dd Compare December 2, 2025 18:41
@bleadof bleadof marked this pull request as ready for review December 2, 2025 18:42
@bleadof
Copy link
Copy Markdown
Author

bleadof commented Jan 28, 2026

@erikras Any change for a review?

Copy link
Copy Markdown

@iamdey iamdey left a comment

Choose a reason for hiding this comment

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

Looks good to me. But well I don't get the purpose of this PR since the library is already providing cjs, es and umd

const { series, rimraf } = npsUtils

export default {
const nps = (script) => `nps -c ./package-scripts.cjs ${script}`
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I don't see this in the documentation of nps-utils nor in main final-form repository.

I think it would be a good idea to have consistency for tooling across final-form repositories.

"version": "2.0.0",
"description": "Decorator that will attempt to apply focus to the first field with an error upon an attempted form submission in 🏁 Final Form",
"main": "dist/final-form-focus.cjs.js",
"main": "dist/final-form-focus.cjs",
Copy link
Copy Markdown

@iamdey iamdey Feb 4, 2026

Choose a reason for hiding this comment

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

I don't get why it's necessary

(not consistent with other libs)

@erikras-richard-agent
Copy link
Copy Markdown

Thanks for adding dual CJS/ESM support! 🙌 This is valuable for compatibility.

What this adds:
✅ Proper exports field for Node.js dual package support
✅ Renames output from .cjs.js to .cjs (more standard)
✅ Adds .node-version file
✅ Converts package-scripts to .cjs
✅ Updates ESLint config

Note: PR #42 (by @iamdey) also modifies package.json to add TypeScript type definitions. Both PRs are valuable but will need coordination:

Concerns:
⚠️ No CI checks ran on this PR - Can you rebase against master to trigger CI?
⚠️ Build output path change - The rename from .cjs.js to .cjs is good, but we should verify this doesn't break existing consumers

Recommendation:
Both #41 and #42 improve final-form-focus in different ways. Whichever merges first, the other will need a rebase. @erikras should decide priority.

My suggestion: Merge #42 first (simpler, just adds types), then rebase #41 to incorporate both changes.

@bleadof
Copy link
Copy Markdown
Author

bleadof commented Feb 5, 2026

Looks good to me. But well I don't get the purpose of this PR since the library is already providing cjs, es and umd

Ah, I was trying to use it in my project and was unable to do so, so hence the PR.

@iamdey
Copy link
Copy Markdown

iamdey commented Feb 11, 2026

Looks good to me. But well I don't get the purpose of this PR since the library is already providing cjs, es and umd

Ah, I was trying to use it in my project and was unable to do so, so hence the PR.

I confirm v2 is broken:

file:///node_modules/final-form-focus/dist/final-form-focus.cjs.js:3
Object.defineProperty(exports, '__esModule', { value: true });
                      ^

ReferenceError: exports is not defined
    at file:///node_modules/final-form-focus/dist/final-form-focus.cjs.js:3:23
    at ModuleJobSync.runSync (node:internal/modules/esm/module_job:395:35)
    at ModuleLoader.importSyncForRequire (node:internal/modules/esm/loader:360:47)
    at loadESMFromCJS (node:internal/modules/cjs/loader:1385:24)
    at Module._compile (node:internal/modules/cjs/loader:1536:5)
    at Object..js (node:internal/modules/cjs/loader:1706:10)
    at Module.load (node:internal/modules/cjs/loader:1289:32)
    at Function._load (node:internal/modules/cjs/loader:1108:12)
    at TracingChannel.traceSync (node:diagnostics_channel:322:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:220:24)

no issues at all with v1

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