Skip to content

Manually define a default export…#5

Merged
savetheclocktower merged 1 commit into
masterfrom
fix-no-default-export
May 4, 2026
Merged

Manually define a default export…#5
savetheclocktower merged 1 commit into
masterfrom
fix-no-default-export

Conversation

@savetheclocktower
Copy link
Copy Markdown

@savetheclocktower savetheclocktower commented May 3, 2026

…in order to play better with transpiled code in the Pulsar codebase.

pulsar-edit#1543 revealed what I should've caught when testing locally: our transpiled-from-MJS version of underscore-plus fails to define a default export. So

import _ from 'underscore-plus`

resulted in _ being undefined. This can be “fixed” by changing all usages to

import * as _ from 'underscore-plus`

but we aren't going to do all that work.

I'm sure there's a better way to fix this, but the way I'm choosing in this PR is to manually export default in the original .mjs file, constructing an object that merges all the underscore methods with all the methods we've added in underscore-plus. This seems to do the right thing when I inspect the transpiled JS, so I'm proceeding to PR.

Once this is approved, we can release @pulsar-edit/underscore-plus version 1.8.2 and update pulsar-edit#1543; that should be enough to fix the 50 different usages that are probably now broken in that PR!

Testing

Once we bump pulsar-edit#1543 we'll find out whether this fixes the root issue. But for the purposes of proving the change in exports, try this:

npm install && npm run prepare
node -e "console.log(typeof require('./lib/underscore-plus').default)"

Run those commands on both master and this PR branch. On master you'll get undefined; on this PR branch you'll get object.

…in order to play better with transpiled code in the Pulsar codebase.
@savetheclocktower
Copy link
Copy Markdown
Author

savetheclocktower commented May 3, 2026

To make this easier to review, I'm trying to figure out some testing instructions. This seems to be a side-effect of our weird use case: originally written using ESM, transpiled to CJS for publishing, but then imported by code that itself is written as though it's ESM but is also being transpiled to CJS!

I will comment again when I can demonstrate the failure on master and prove that it is fixed on this PR branch. Updated the testing instructions in the description!

@savetheclocktower
Copy link
Copy Markdown
Author

I'll probably merge this by the end of the day if it doesn't get any reviews… on the logic that (a) this change self-evidently doesn't regress anything (since CI is green); and (b) it's arguably just a bookkeeping step rather than something that changes the behavior of the library.

In 1.8.1, this library defines no default export; in this PR branch, a default export is present, and that change cannot conceivably regress anything anywhere. But I do want to at least give people the opportunity to look it over and suggest different approaches!

Copy link
Copy Markdown
Member

@DeeDeeG DeeDeeG left a comment

Choose a reason for hiding this comment

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

Lite approve 👍, I don't pretend to fully understand all of this, but I can confirm the testing steps at the bottom of the PR description behave as described in the before/after.

I hope it works as intended over at core repo!

@savetheclocktower savetheclocktower merged commit 87bfae0 into master May 4, 2026
6 checks passed
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