Skip to content
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

fix: Do not delete files before all upload tasks executed #572

Merged
merged 4 commits into from
Jul 12, 2024

Conversation

tyouzu1
Copy link
Contributor

@tyouzu1 tyouzu1 commented Jul 11, 2024

#570

writeBundle object dont support vite v2, should solve real problems, the dependenciesAreFreedPromise has been used and will not wait a second time

In order to ensure that each deletion task will wait normally, it is necessary to recreate the dependenciesAreFreedPromise each time

@tyouzu1
Copy link
Contributor Author

tyouzu1 commented Jul 11, 2024

@lforst
I don't have enough time to test other frameworks(like webpack).
But I think you should probably upgrade unplugin, I don't know if this has any impact,
unjs/unplugin#389

So I kept the original code logic, maybe it will look strange

const freeInitDependencyOnSourcemapFiles = createDependencyOnSourcemapFiles();
...
const freeDependencyOnSourcemapFiles = createDependencyOnSourcemapFiles();
...
freeInitDependencyOnSourcemapFiles();
freeDependencyOnSourcemapFiles();

Really sorry for the last change.

@lforst
Copy link
Member

lforst commented Jul 12, 2024

Thanks a bunch for the PR! Unfortunately, I don't understand how this PR will resolve the original problem. Would you mind adding a test case to highlight what exactly is fixed?

@tyouzu1
Copy link
Contributor Author

tyouzu1 commented Jul 12, 2024

Thanks a bunch for the PR! Unfortunately, I don't understand how this PR will resolve the original problem. Would you mind adding a test case to highlight what exactly is fixed?

With the existing code, it seems that adding test cases requires a lot of effort.
I don't dare to make too many changes, Or do you just need a demo?

@lforst
Copy link
Member

lforst commented Jul 12, 2024

@tyouzu1 if you're not comfortable adding tests, a demo would also be fine!

@lforst
Copy link
Member

lforst commented Jul 12, 2024

@tyouzu1 actually all good. Thanks to your explanation in the issue I grokked it now! Thank you!

@tyouzu1
Copy link
Contributor Author

tyouzu1 commented Jul 12, 2024

@tyouzu1 actually all good. Thanks to your explanation in the issue I grokked it now! Thank you!

thank you for your patience

@lforst lforst changed the title fix: Resolve the deletion task order fix: Do not delete files before _all_ upload tasks executed Jul 12, 2024
@lforst lforst changed the title fix: Do not delete files before _all_ upload tasks executed fix: Do not delete files before *all* upload tasks executed Jul 12, 2024
@lforst lforst changed the title fix: Do not delete files before *all* upload tasks executed fix: Do not delete files before all upload tasks executed Jul 12, 2024
@lforst lforst merged commit e66910d into getsentry:main Jul 12, 2024
17 checks passed
@tyouzu1
Copy link
Contributor Author

tyouzu1 commented Jul 15, 2024

demo, the write hook will be called twice

vite.build({
  build: {
    sourcemap: true,
    rollupOptions: {
      output: {
        format: "cjs",
        entryFileNames: "[name].js",
      },
    },
  },
  plugins: [
    {
      name: "multi-write-bundle-test",
      enforce: "post",
      configResolved({ build: { rollupOptions } }) {
        rollupOptions.output = [
          {
            format: "system",
            entryFileNames: "[name]-plugin.js",
          },
          {
            format: "cjs",
            entryFileNames: "[name].js",
          },
        ];
      },
      generateBundle(options, bundle) {
        if (options.format === "system") {
          for (const name in bundle) {
            if (bundle[name]?.type === "asset" && !/.+\.map$/.test(name)) {
              delete bundle[name];
            }
          }
        }
      },
    },
    sentryVitePlugin({
      debug: true,
      org: "xx",
      project: "xx",
      authToken:
        "xxx",
      sourcemaps: {
        filesToDeleteAfterUpload: [`dist/**/*.js.map`],
      },
    }),
  ],
});
dist/index-plugin.js  142.33 kB │ gzip: 45.67 kB │ map: 347.64 kB
[sentry-vite-plugin] Info: Successfully uploaded source maps to Sentry
[sentry-vite-plugin] Debug: Deleting asset after upload: dist/index-plugin.js.map
[sentry-vite-plugin] Debug: Deleting asset after upload: dist/index.js.map
dist/index.html    0.37 kB │ gzip:  0.26 kB
dist/index.js    142.25 kB │ gzip: 45.63 kB │ map: 347.65 kB
[sentry-vite-plugin] Info: Successfully uploaded source maps to Sentry

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants