ref(build): Handle minification in rollup config generation function #4681
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
size-limit report
|
lobsterkatie
force-pushed
the
kmclb-handle-minification-in-rollup-function
branch
from
6f5727e to
261d1e8
Compare
AbhiPrasad
approved these changes
Mar 4, 2022
rollup.config.js
Outdated
Comment on lines
22
to
23
| } | ||
| export function insertAt(arr, index, insertee) { |
There was a problem hiding this comment.
This doesn't prettier?
Suggested change
| } | |
| export function insertAt(arr, index, insertee) { | |
| } | |
| export function insertAt(arr, index, insertee) { |
There was a problem hiding this comment.
Do you mean, does prettier get mad that there's not a blank line there? No - it would have failed linting. My thinking in not putting a blank there is I wanted the comment to apply to both, and it felt like they were both small enough I could get away with it. But you're not wrong, it's a little weird looking. Compromise option:
/**
* Helper functions to compensate for the fact that JS can't handle negative array indices very well
*
* TODO `insertAt` is only exported so the integrations config can inject the `commonjs` plugin, for localforage (used
* in the offline plugin). Once that's fixed to no longer be necessary, this can stop being exported.
*/
const getLastElement = array => {
return array[array.length - 1];
};
export const insertAt = (arr, index, insertee) => {
const newArr = [...arr];
// Add 1 to the array length so that the inserted element ends up in the right spot with respect to the length of the
// new array (which will be one element longer), rather than that of the current array
const destinationIndex = index >= 0 ? index : arr.length + 1 + index;
newArr.splice(destinationIndex, 0, insertee);
return newArr;
};Functionally* the same, but having two consts in a row is better. I'll switch it to that.
* no pun intended, I swear
lobsterkatie
force-pushed
the
kmclb-handle-minification-in-rollup-function
branch
from
261d1e8 to
f8b436e
Compare
lobsterkatie
force-pushed
the
kmclb-handle-minification-in-rollup-function
branch
from
f8b436e to
6b30492
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
As part of the new build process, this removes minification config from being hardcoded in individual
rollup.config.jsfiles and instead uses a new function in the mainrollup.config.jsto generate minified and unminified variants of each build config it's given.This PR also includes a few other small changes:
rollup.config.jsfiles to have more parallel structure.markAsBrowserBulidplugin so it has the work "plugin" in its name.Ref: https://getsentry.atlassian.net/browse/WEB-633