fix: terminate global shim banner statements#162
Open
SanderMuller wants to merge 1 commit into
Open
Conversation
The dev banner is prepended to every optimized dependency. Its lines had no
trailing semicolons, so when a dependency began with a UMD/IIFE (its first
character is `(` or `+`), ASI merged the banner's last assignment with it:
globalThis.process = globalThis.process || __process_polyfill + function () {}()
Since the global is already defined, || short-circuits and the IIFE never runs,
so libraries like bootstrap and jquery-ui silently fail to initialize under the
rolldown/esbuild dep optimizer. Terminating each statement keeps the dependency
a separate statement.
Fixes davidmyersdev#155
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Fixes #155.
The bug
The banner prepended to every optimized dependency has no semicolons terminating its lines. If a dependency's first line starts with
(or+(any UMD/IIFE bundle, e.g. bootstrap or jquery-ui), ASI won't insert a semicolon there, so the banner's last assignment merges with the dependency:+and the call bind tighter than||, andglobalThis.processis already defined by the time the dep runs, so||short-circuits and the IIFE never executes. The library's plugins/widgets never register, and there's no error at load. It surfaces later as$(...).tooltip is not a function,.draggable is not a function, and similar.Reported in #155, and originally in vitejs/vite#22167 where @sapphi-red confirmed it's this plugin's bug. #152 moved the main injection off
esbuild.banner, but theoptimizeDepspre-bundle still emits this banner on both the rolldown and esbuild branches, so dependency pre-bundling is still affected on 0.28.0.The fix
Terminate each statement in
globalShimBanners. Every line is now a complete statement, so a following dependency can't merge into it.Tests
test/unit/global-shim-banner.test.ts:+function(){}()with the global already defined, assert the IIFE still runs (red on the old banner, green with the fix)Unit suite, typecheck, and lint pass.