feat: Add in bundle name config option #26

nicholas-codecov · 2023-12-13T19:54:32Z

Description

This PR adds in a new configuration option bundleName which is required when using the bundle analysis plugin. If not provided then an error message will be emitted and the core plugin will not attempt and upload.

Code Example

codecovVitePlugin({
  enableBundleAnalysis: true,
  bundleName: "vite-test",
}),

Notable Changes

Add new bundleName config option
Append bundleName to stats JSON output
Update bundler plugins to early return and emit console error message
Update tests

codecov · 2023-12-13T19:57:17Z

Codecov Report

Attention: 20 lines in your changes are missing coverage. Please review.

Comparison is base (7fc27b3) 85.81% compared to head (3930adc) 84.56%.

Files	Patch %	Lines
...ack-bundle-analysis/webpackBundleAnalysisPlugin.ts	12.50%	6 Missing and 1 partial ⚠️
...llup-bundle-analysis/rollupBundleAnalysisPlugin.ts	0.00%	6 Missing ⚠️
...c/vite-bundle-analysis/viteBundleAnalysisPlugin.ts	0.00%	6 Missing ⚠️
...src/bundle-analysis/bundleAnalysisPluginFactory.ts	50.00%	1 Missing ⚠️

Additional details and impacted files

Components	Coverage Δ
Plugin core	`96.26% <50.00%> (-0.09%)`	⬇️
Rollup plugin	`7.93% <0.00%> (+0.79%)`	⬆️
Vite plugin	`7.93% <0.00%> (+0.79%)`	⬆️
Webpack plugin	`9.80% <12.50%> (+0.50%)`	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

codecov

The changes in the git diff seem to be about the addition of a new bundleName option to the bundling plugins. The bundleName is set in the tests, function calls, and is added to the Output interface, but there is no clear use of it in the plugins themselves except for some condition checks and warning logging. The description or purpose behind this bundleName addition is not clear from the git diff alone and it could be helpful to provide more information on its usage.

codecov · 2023-12-13T19:58:24Z

integration-tests/fixtures/generate-bundle-stats/rollup-plugin.test.ts

+        codecovRollupPlugin({
+          enableBundleAnalysis: true,
+          dryRun: true,
+          bundleName: "rollup-test",


The bundleName option is added to the plugin, but it is not clear what it means or how it is used within the plugin itself. It could be helpful to add comments explaining what it does.

codecov · 2023-12-13T19:58:24Z

integration-tests/fixtures/generate-bundle-stats/rollup-plugin.test.ts

@@ -89,4 +93,8 @@
  it("sets the correct bundler information", () => {
    expect(stats.bundler).toStrictEqual(expectedStats.bundler);
  });
+
+  it("sets the correct bundle name", () => {


The test checks if the bundleName in the stats is correctly set to the one provided in the plugin options. However, it is not clear how this bundleName is used in the test or the functionality it provides.

codecov · 2023-12-13T19:58:24Z

packages/bundler-plugin-core/src/bundle-analysis/bundleAnalysisPluginFactory.ts

  const output: Output = {
    version: "1",
+    bundleName: userOptions.bundleName ?? "",
  };


The bundleName is added to the Output object here. It would be useful to know where this object is used and what effect the bundleName has on it.

codecov · 2023-12-13T19:58:24Z

packages/bundler-plugin-core/src/index.ts

@@ -21,7 +21,7 @@ interface CodecovUnpluginFactoryOptions {
  bundleAnalysisUploadPlugin: BundleAnalysisUploadPlugin;
 }

-export function codecovUnpluginFactory({
+function codecovUnpluginFactory({
  bundleAnalysisUploadPlugin,


The codecovUnpluginFactory function is sightly modified, and it's not clear why. Perhaps it is related to the bundleName implementation but it needs clarification.

codecov · 2023-12-13T19:58:24Z

packages/bundler-plugin-core/src/types.ts

@@ -28,6 +28,7 @@ export interface Module {

 export interface Output {
  version?: string;
+  bundleName: string;


The bundleName field is added to the Output Interface. But, It seems to be mandatory in the Interface which might not be sensible as it's optional in Options Interface (and can be undefined).

codecov · 2023-12-13T19:58:24Z

packages/bundler-plugin-core/src/types.ts

+   * Required for uploading bundle analysis information.
+   *
+   * Example: `rollup-package`
+   */


The bundleName field is added to the Options Interface, but it's marked as optional. It should be mandatory if it's required for uploading bundle analysis information.

codecov · 2023-12-13T19:58:25Z

packages/rollup-plugin/src/rollup-bundle-analysis/rollupBundleAnalysisPlugin.ts

@@ -17,6 +18,12 @@
  pluginVersion: "1.0.0",
  rollup: {
    generateBundle(this, options, bundle) {
+      // don't need to do anything if the bundle name is not present or empty
+      if (!userOptions.bundleName || userOptions.bundleName === "") {


If bundleName is not provided or is an empty string, red text stating 'Bundle name is not present or empty. Skipping upload.' is logged to the console. However, it's not sure if this behavior of skipping upload when bundleName is missing suits all use-cases.

codecov · 2023-12-13T19:58:25Z

packages/vite-plugin/src/vite-bundle-analysis/viteBundleAnalysisPlugin.ts

+        red("Bundle name is not present or empty. Skipping upload.");
+        return;
+      }
+
      const customOptions = {


Again here, the upload is skipped if bundleName is not present. There seems to be a pattern of operational changes depending on whether the bundleName exists.

codecov · 2023-12-13T19:58:25Z

packages/webpack-plugin/src/webpack-bundle-analysis/webpackBundleAnalysisPlugin.ts

-import { type BundleAnalysisUploadPlugin } from "@codecov/bundler-plugin-core";
+import {
+  type BundleAnalysisUploadPlugin,
+  red,


Red is imported but unused. This needs to be reviewed and removed if it's not needed.

codecov · 2023-12-13T19:58:25Z

packages/webpack-plugin/src/webpack-bundle-analysis/webpackBundleAnalysisPlugin.ts

@@ -18,6 +21,12 @@
          stage: webpack4or5.Compilation.PROCESS_ASSETS_STAGE_REPORT,
        },
        () => {
+          // don't need to do anything if the bundle name is not present or empty
+          if (!userOptions.bundleName || userOptions.bundleName === "") {


Without the context of what this bundleName is meant to contribute to the functionality of the plugins, the addition of it at so many points throughout these files lessens the readability and understanding.

codecov

CodecovAI submitted a new review for 3930adc

packages/bundler-plugin-core/src/bundle-analysis/bundleAnalysisPluginFactory.ts

packages/bundler-plugin-core/src/bundle-analysis/__tests__/bundleAnalysisPluginFactory.test.ts

packages/vite-plugin/src/vite-bundle-analysis/viteBundleAnalysisPlugin.ts

packages/bundler-plugin-core/src/types.ts

nicholas-codecov added 6 commits December 13, 2023 14:33

add bundle name config option to bundler core

4b1474e

update unit tests

26098f0

update integration tests

ccd55ed

make bundle name optional for future incase we expand our plugins

f37a830

early return so we don't attempt to upload

df6652b

emit warning from the various bundler plugins

2da2a19

codecov bot reviewed Dec 13, 2023

View reviewed changes

scott-codecov approved these changes Dec 13, 2023

View reviewed changes

nicholas-codecov added 3 commits December 14, 2023 12:18

append bundler name option if present

acba306

append output format to bundle name

4f9e05b

update integration tests

92ae7b4

AbhiPrasad approved these changes Dec 18, 2023

View reviewed changes

Merge branch 'main' into add-in-bundle-name-config-option

3930adc

codecov bot reviewed Dec 19, 2023

View reviewed changes

nicholas-codecov merged commit 78966dc into main Dec 19, 2023
7 of 12 checks passed

nicholas-codecov deleted the add-in-bundle-name-config-option branch December 19, 2023 11:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add in bundle name config option #26

feat: Add in bundle name config option #26

nicholas-codecov commented Dec 13, 2023

codecov bot commented Dec 13, 2023 •

edited

Loading

codecov bot left a comment

codecov bot Dec 13, 2023

codecov bot Dec 13, 2023

codecov bot Dec 13, 2023

codecov bot Dec 13, 2023

codecov bot Dec 13, 2023

codecov bot Dec 13, 2023

codecov bot Dec 13, 2023

codecov bot Dec 13, 2023

codecov bot Dec 13, 2023

codecov bot Dec 13, 2023

codecov bot left a comment

feat: Add in bundle name config option #26

feat: Add in bundle name config option #26

Conversation

nicholas-codecov commented Dec 13, 2023

Description

Code Example

Notable Changes

codecov bot commented Dec 13, 2023 • edited Loading

Codecov Report

codecov bot left a comment

Choose a reason for hiding this comment

codecov bot Dec 13, 2023

Choose a reason for hiding this comment

codecov bot Dec 13, 2023

Choose a reason for hiding this comment

codecov bot Dec 13, 2023

Choose a reason for hiding this comment

codecov bot Dec 13, 2023

Choose a reason for hiding this comment

codecov bot Dec 13, 2023

Choose a reason for hiding this comment

codecov bot Dec 13, 2023

Choose a reason for hiding this comment

codecov bot Dec 13, 2023

Choose a reason for hiding this comment

codecov bot Dec 13, 2023

Choose a reason for hiding this comment

codecov bot Dec 13, 2023

Choose a reason for hiding this comment

codecov bot Dec 13, 2023

Choose a reason for hiding this comment

codecov bot left a comment

Choose a reason for hiding this comment

codecov bot commented Dec 13, 2023 •

edited

Loading