Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Conditional Exports in Popper v2 #1526

Closed
wants to merge 3 commits into from
Closed

Add Conditional Exports in Popper v2 #1526

wants to merge 3 commits into from

Conversation

chadrschroeder
Copy link

@chadrschroeder chadrschroeder commented Feb 3, 2022

Fixes #1465.

This pull request is similar to #1502, but for the v2.x branch.

I've left all existing distributions in place and have added 2 new files to use in the development and production conditional exports:

Exports Entry File Name Notes
import.development dist/esm/index.development.js New un-minified ESM build with debugging on.
import.production dist/esm/index.min.js New minified ESM build with debugging off.
import.default lib/index.js Existing ESM build. Same as module.
require dist/cjs/popper.js Existing CJS build. Same as main.

I copied the entire "exports" format from the current version which means I also brought back the update from #1462. I can leave that out if you'd prefer.

Please let me know if I'm doing anything wrong here or if there's a different approach you would recommend.

@rollingversions
Copy link

There is no change log for this pull request yet.

Create a changelog

format === 'esm' && development &&
replace({
'process.env.NODE_ENV': '"development"',
}),
Copy link
Author

Choose a reason for hiding this comment

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

Because I didn't want to alter anything about the existing build, __DEV__ will still be replaced by babel-plugin-dev-expression. Then I replace the process.env.NODE_ENV reference after that.

@dominikg
Copy link

this was added before #1342 and rolled back #1362 due to it being a breaking change. (adding an exports map always is)

I think it is worth supporting esm with export maps but it would have to be in a new major release.

@atomiks
Copy link
Collaborator

atomiks commented May 28, 2022

Based on Rich's comment here: sveltejs/kit#4504 (comment)

Would it be enough to create a new mjs folder build? esm used to have .mjs as the extension name, but it changed to .js in v2.0.4 for some reason — at this point, it's probably too major of a breaking change to revert that back to .mjs, right? I don't think we can add an export map at all though.

But we could add a new folder using that extension. Libraries could then import from that folder instead of /esm, which would basically be the same as a new major.

@chadrschroeder
Copy link
Author

I'm going to close this PR. I'm OK with sticking with how V2 works at this point. If someone else has a different need that the new mjs folder would help out with, I'll let them go for it in another PR.

Thank you for working on the export maps in the current version of floating-ui! I'm looking forward to when I'll be able to use that.

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.

None yet

3 participants