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

fix: inline findUpSync util for CJS compatibility #17

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

fmal
Copy link
Contributor

@fmal fmal commented Jan 27, 2025

Description

This PR solves #16 by inlining simplified findUpSync util that was previously imported from find-up-simple (https://github.com/sindresorhus/find-up-simple/blob/main/index.js#L32). This caused issues because eslint-config-flat-gitignore was unusable in CJS environment because find-up-simple package is ESM-only.

Nice side effect is that there is only one external dependency now.

Linked Issues

Closes #16

@antfu
Copy link
Owner

antfu commented Jan 28, 2025

I am actually starting do drop CJS at a whole - I wonder what's the problem that preventing you from going to ESM?

@antfu
Copy link
Owner

antfu commented Jan 28, 2025

Ok, I see you explained in #16, it's indeed a bug that the CJS is provided but not working. I think I will have your PR and release a patch, while soon going to a new major that drops CJS (so if you still need CJS you can be on 1.x). Does that work for you?

@fmal
Copy link
Contributor Author

fmal commented Jan 28, 2025

@antfu yeah, that sounds like a good solution (fixing the CJS compatibility on 1.x and then releasing a major that drops it if you wish to do so). Thanks a lot!

I am actually starting do drop CJS at a whole - I wonder what's the problem that preventing you from going to ESM?

My sharable config is also meant to be consumed by some older projects not yet ready for ESM.

@antfu antfu merged commit 085e2b8 into antfu:main Jan 28, 2025
5 checks passed
@antfu
Copy link
Owner

antfu commented Jan 28, 2025

My sharable config is also meant to be consumed by some older projects not yet ready for ESM.

If they are already on the flat config, they can just rename the config to .mjs and nothing need to change otherwise.

@fmal
Copy link
Contributor Author

fmal commented Jan 28, 2025

My sharable config is also meant to be consumed by some older projects not yet ready for ESM.

If they are already on the flat config, they can just rename the config to .mjs and nothing need to change otherwise.

That's a good point. My config is currently hybrid package built with https://github.com/isaacs/tshy (similar to how you used unbuild). But i think in the future i might also drop the hybrid approach and add a note about adding .mjs extension to force ESM.

Thanks for merging ❤️

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.

CommonJS compatibility
2 participants