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

Reenable Rollup preserveModules option, and consolidate utils/utilities directories. #5437

Merged
merged 26 commits into from
Oct 11, 2019

Conversation

hwillson
Copy link
Member

This PR does the following:

  • Re-enables the rollup preserveModules feature, to make sure ESM files are modified in place (instead of being bundled) when running the invariantPlugin.
  • Removes all module index.ts files, to stop confusing rollup when using the preserveModules feature.
  • Combines /utils/, /utilities, and /utilities/utils into a more logical utility directory structure.
  • Removes exports from index.ts that we no longer want to be available externally.
  • Moves test only utilities out of the main bundle.

With `preserveModules` off, rollup bundles the ESM code up and
stores it in `dist/index.js`, when our first configured rollup
job runs. This job is intended to run the `invariantPlugin`
against all ESM code, updating `InvariantError`'s in place. It
shouldn't be storing the results of running the `invariantPlugin`
in `dist/index.js`. Enabling `preserveModules` ensures that
changes are written back into originating files.
Importing from module index files causes an issue with rollup
when using `preserveModules`, such that rollup expands index
imports in files to include extra imports that aren't needed.
Since we don't really need index files, let's just get rid of
them.
Let's get these back to being internal utilities (to help reduce
our public API).
@hwillson hwillson self-assigned this Oct 11, 2019
@hwillson hwillson added this to the Release 3.0 milestone Oct 11, 2019
Copy link
Member

@benjamn benjamn left a comment

Choose a reason for hiding this comment

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

This looks great to me! I have some commits on my branch that I'd like to cherry-pick on top of this, to avoid merge conflicts later, so I can either push more commits to the PR, or we can go ahead and merge it into release-3.0.

package.json Show resolved Hide resolved
config/rollup.config.js Show resolved Hide resolved
src/__tests__/utils/observableToPromise.ts Outdated Show resolved Hide resolved
@benjamn benjamn changed the title More bundling adjustments Reenable Rollup preserveModules option, and consolidate utils/utilities directories. Oct 11, 2019
@benjamn benjamn mentioned this pull request Oct 11, 2019
31 tasks
@hwillson hwillson merged commit a8abe35 into release-3.0 Oct 11, 2019
@hwillson hwillson deleted the rollup-related-improvements branch October 11, 2019 16:20
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants