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

feat: support disabling preserveDynamicImports #322

Merged
merged 6 commits into from
Oct 30, 2023
Merged

Conversation

brendonmatos
Copy link
Contributor

@brendonmatos brendonmatos commented Sep 27, 2023

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme, or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

This PR adds support to choose to turn off "preserve dynamic imports" to older environments.
Issue found using unbuild to build a dependency of create-react-app v4 project where it's doesn't support esm import

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@pi0
Copy link
Member

pi0 commented Sep 27, 2023

Thanks for this pull request. But since this is mostly an edge-case for legacy environments not supporting dynamic imports (not sure which not version) would it makes sense that we document it as how to configure rollup via hooks?

@brendonmatos
Copy link
Contributor Author

brendonmatos commented Oct 5, 2023

Well, to be honest, I don't know which is the right way to go.

But, I understood that other options follow the same, like:

unbuild/src/types.ts

Lines 48 to 49 in 1d06a26

emitCJS?: boolean;
cjsBridge?: boolean;

Do you think that this option differs too much from others?

@pi0
Copy link
Member

pi0 commented Oct 30, 2023

Other options are often to "opt-in" into a behavior while this one is an "opt-out" for an expected behavior. I am not against introducing an option to make unbuild customizable only questioning if this is really something we expect to be normally used.

Update: Let's have it! I have renamed option name slightly to reflect that it always changes behavior.

Issue found using unbuild to build a dependency of create-react-app v4 project where it's doesn't support esm import

Do you have more information of this? What library supports legacy node without dynamic import support?

@pi0 pi0 changed the title feat: support turning off preserve dynamic imports cjs feat: support disabling preserveDynamicImports Oct 30, 2023
@pi0 pi0 merged commit 2e487c3 into unjs:main Oct 30, 2023
2 checks passed
@brendonmatos
Copy link
Contributor Author

Other options are often to "opt-in" into a behavior while this one is an "opt-out" for an expected behavior. I am not against introducing an option to make unbuild customizable only questioning if this is really something we expect to be normally used.

Update: Let's have it! I have renamed option name slightly to reflect that it always changes behavior.

Issue found using unbuild to build a dependency of create-react-app v4 project where it's doesn't support esm import

Do you have more information of this? What library supports legacy node without dynamic import support?

Sorry for the late. But I was using this library to build a team lib that is currently using material-ui 4.

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