Feature/fix sourcemap url#299
Merged
Merged
Conversation
- Add support for `sourceMapBaseUrl` in sourcemap configuration. - Use `path.basename` for consistent source map file names across utilities.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts how sourcemap filenames and base URLs are generated for obfuscated bundles, and updates documentation branding. Sequence diagram for updated sourcemap configuration and obfuscationsequenceDiagram
actor Dev
participant Vite as ViteBuild
participant Plugin as viteBundleObfuscator
participant Utils as obfuscateBundle
participant Obf as javascriptObfuscator
Dev->>Vite: Run build with sourcemap enabled
Vite->>Plugin: Call configResolvedHandler(resolvedConfig)
Plugin->>Plugin: Read resolvedConfig.build.sourcemap
Plugin->>Plugin: Read rollupOptions.output.sourcemapBaseUrl
Plugin->>Plugin: Set finalConfig.options.sourceMap = true
Plugin->>Plugin: Set finalConfig.options.sourceMapMode
Plugin->>Plugin: Set finalConfig.options.sourceMapBaseUrl (if defined)
Vite->>Plugin: Generate bundle and invoke obfuscation
Plugin->>Utils: obfuscateBundle(finalConfig, fileName, bundleItem)
Utils->>Utils: fileSpecificOptions = {
Utils->>Utils: ...finalConfig.options,
Utils->>Utils: inputFileName = fileName,
Utils->>Utils: sourceMapFileName = path.basename(fileName) + ".map"
Utils->>Obf: obfuscate(bundleItem.code, fileSpecificOptions)
Obf-->>Utils: obfuscated code with sourcemap
Utils-->>Plugin: obfuscated bundle
Plugin-->>Vite: final obfuscated assets with correct sourcemap URLs
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In
configResolvedHandler,resolvedConfig.build.rollupOptions?.outputcan be an array; if multiple outputs are configured, consider either handling the array case or explicitly validating/throwing so behavior is predictable rather than silently ignoring it. - When setting
sourceMapFileNamewithpath.basename(fileName), double-check that this matches Vite/Rollup’s final emitted filenames (especially when using hashes or customentryFileNamespatterns), otherwise sourcemap URLs might not align with the generated bundle names.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `configResolvedHandler`, `resolvedConfig.build.rollupOptions?.output` can be an array; if multiple outputs are configured, consider either handling the array case or explicitly validating/throwing so behavior is predictable rather than silently ignoring it.
- When setting `sourceMapFileName` with `path.basename(fileName)`, double-check that this matches Vite/Rollup’s final emitted filenames (especially when using hashes or custom `entryFileNames` patterns), otherwise sourcemap URLs might not align with the generated bundle names.
## Individual Comments
### Comment 1
<location> `src/index.ts:111-112` </location>
<code_context>
const configResolvedHandler: (resolvedConfig: ResolvedConfig) => void | Promise<void> = (resolvedConfig) => {
const sourcemap = resolvedConfig.build.sourcemap;
if (sourcemap) {
+ const output = resolvedConfig.build.rollupOptions?.output;
+ const sourcemapBaseUrl = !isArray(output) ? output?.sourcemapBaseUrl : undefined;
finalConfig.options = {
...finalConfig.options,
</code_context>
<issue_to_address>
**suggestion:** `sourcemapBaseUrl` is ignored when `rollupOptions.output` is an array, which might be surprising for multi-output builds.
Because of the `!isArray(output)` guard, `sourcemapBaseUrl` is effectively unsupported for multi-output Rollup configs, even when all outputs could share the same base URL. If multi-output builds are a target use case, consider either handling the array case (e.g., ensure all outputs have the same `sourcemapBaseUrl` and use it) or clearly documenting that this option only applies to single-output configs.
</issue_to_address>
### Comment 2
<location> `src/index.ts:112` </location>
<code_context>
const sourcemapBaseUrl = !isArray(output) ? output?.sourcemapBaseUrl : undefined;
</code_context>
<issue_to_address>
**suggestion (code-quality):** Invert ternary operator to remove negation ([`invert-ternary`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/invert-ternary))
```suggestion
const sourcemapBaseUrl = isArray(output) ? undefined : output?.sourcemapBaseUrl;
```
<br/><details><summary>Explanation</summary>Negated conditions are more difficult to read than positive ones, so it is best
to avoid them where we can. By inverting the ternary condition and swapping the
expressions we can simplify the code.
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+111
to
+112
| const output = resolvedConfig.build.rollupOptions?.output; | ||
| const sourcemapBaseUrl = !isArray(output) ? output?.sourcemapBaseUrl : undefined; |
There was a problem hiding this comment.
suggestion: sourcemapBaseUrl is ignored when rollupOptions.output is an array, which might be surprising for multi-output builds.
Because of the !isArray(output) guard, sourcemapBaseUrl is effectively unsupported for multi-output Rollup configs, even when all outputs could share the same base URL. If multi-output builds are a target use case, consider either handling the array case (e.g., ensure all outputs have the same sourcemapBaseUrl and use it) or clearly documenting that this option only applies to single-output configs.
| const sourcemap = resolvedConfig.build.sourcemap; | ||
| if (sourcemap) { | ||
| const output = resolvedConfig.build.rollupOptions?.output; | ||
| const sourcemapBaseUrl = !isArray(output) ? output?.sourcemapBaseUrl : undefined; |
There was a problem hiding this comment.
suggestion (code-quality): Invert ternary operator to remove negation (invert-ternary)
Suggested change
| const sourcemapBaseUrl = !isArray(output) ? output?.sourcemapBaseUrl : undefined; | |
| const sourcemapBaseUrl = isArray(output) ? undefined : output?.sourcemapBaseUrl; |
Explanation
Negated conditions are more difficult to read than positive ones, so it is bestto avoid them where we can. By inverting the ternary condition and swapping the
expressions we can simplify the code.
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.
Summary by Sourcery
Update source map handling to better align obfuscated bundle outputs with Vite/Rollup sourcemap configuration.
Bug Fixes:
Enhancements: