Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
implement bundling changes suggested in #4062 #4096
base: 16.x.x
Are you sure you want to change the base?
implement bundling changes suggested in #4062 #4096
Changes from 5 commits
85ecc7a
9646d84
16fa5d5
2fa37dc
881b1bf
864015c
f93c7bf
011ce2c
f86e6d0
73adce6
44516d2
bea8025
3f74ec5
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These have been chosen by gut feeling out of the exports mentioned in #4074
We need to generally decide on a way forward here:
.js
file?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Labelling a function with
export
but burying it so users would have to deep import it was a way to allow entryway into our private API without issuing any semver guarantee => a convenience to power users, so to speak.With explicit exports, with option 1 and 2, we kind of lose that distinction, and it's much more difficult to garden off these bits as a private API that happens to be exposed.
So I vote that we examine each in term, and make the somewhat difficult decision of whether to support semver on them, and in that case go with option C, export from main entrypoint, or no longer expose them at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I feel similarly.
I would suggest that I remove all of these exports and the annotations I made in this PR, and we open a new PR to expose these after deciding which we want to expose?