Skip to content

chore: distribute cjs @W-21382713#5719

Merged
wjhsf merged 1 commit intomasterfrom
wjh/oops-distribute-cjs
Feb 27, 2026
Merged

chore: distribute cjs @W-21382713#5719
wjhsf merged 1 commit intomasterfrom
wjh/oops-distribute-cjs

Conversation

@wjhsf
Copy link
Contributor

@wjhsf wjhsf commented Feb 27, 2026

I intended to do this with the v9.0.0 release, but apparently forgot.

Details

Does this pull request introduce a breaking change?

  • 😮‍💨 No, it does not introduce a breaking change.
  • 💔 Yes, it does introduce a breaking change.

Does this pull request introduce an observable change?

  • 🤞 No, it does not introduce an observable change.
  • 🔬 Yes, it does include an observable change.

GUS work item

intended to do this with the v9.0.0 release, but apparently forgot
@wjhsf wjhsf requested a review from a team as a code owner February 27, 2026 16:49
@wjhsf wjhsf changed the title chore: distribute cjs chore: distribute cjs @W-21382713 Feb 27, 2026
Copy link
Contributor

@rax-it rax-it left a comment

Choose a reason for hiding this comment

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

@wjhsf
Do we need something like:

"exports": {
  ".": {
    "import": "./dist/index.js",
    "require": "./dist/index.cjs"
  }
}

For lwc-transformer to import it correctly? I'm not sure how the .cjs file is being resolved?

@wjhsf
Copy link
Contributor Author

wjhsf commented Feb 27, 2026

I tried adding exports as part of the original v9 release, but it broke too many other things. For v9, dist/index.cjs exists. For v8, dist/index.cjs doesn't exist, so node goes through its list of assumed file extensions. dist/index.cjs.js does exist, so the import works. (There's no special meaning to the .cjs in the file name. Node treats it the same way it would treat dist/index or dist/foo.)

Copy link
Contributor

@rax-it rax-it left a comment

Choose a reason for hiding this comment

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

Looks like in v8 we had:

"main": "dist/index.cjs.js",
"module": "dist/index.js",

which is probably why node was able to resolve it, and we don't have this in v9.
Sorry, I am still not convinced that this will work.
Can you link this locally to lwc-test and make sure it works?

@wjhsf
Copy link
Contributor Author

wjhsf commented Feb 27, 2026

The "module" field is a convention for bundlers; node itself doesn't use it in the lookup algorithm. require("@lwc/engine-dom/dist/index.cjs") works for both v8 and v9.

@rax-it
Copy link
Contributor

rax-it commented Feb 27, 2026

The "module" field is a convention for bundlers; node itself doesn't use it in the lookup algorithm. require("@lwc/engine-dom/dist/index.cjs") works for both v8 and v9.

Sure that will work but will this import without nested import work?
Do we need to update the jest-transformer?

I don't think explicitly requiring .cjs extension is the solution but as you mentioned it was causing other issues so I am willing to give this a shot.

@wjhsf
Copy link
Contributor Author

wjhsf commented Feb 27, 2026

Sure that will work but will this import without nested import work?

No, this change will not make require("@lwc/compiler") work, but that is not the goal of this PR. The goal of this PR is simply to publish .cjs files. To fix the issues with lwc-test, there are other changes in salesforce/lwc-test#404.

@wjhsf wjhsf merged commit bb244a6 into master Feb 27, 2026
47 of 48 checks passed
@wjhsf wjhsf deleted the wjh/oops-distribute-cjs branch February 27, 2026 21:51
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.

2 participants