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: add sourcemap file to chunk.auxiliaryFiles #6125

Merged
merged 5 commits into from
Apr 18, 2024

Conversation

hulin32
Copy link
Contributor

@hulin32 hulin32 commented Apr 6, 2024

Summary

fix: add sourcemap file to chunk.auxiliaryFiles (#3253), follow up for #5490

Test Plan

I created a demo to test it
https://github.com/hulin32/rspack-for-react-test-verification

Require Documentation?

  • No
  • Yes, the corresponding rspack-website PR is __

Copy link

netlify bot commented Apr 6, 2024

Deploy Preview for rspack canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit d886fc0
🔍 Latest deploy log https://app.netlify.com/sites/rspack/deploys/66208e2db9a82c00082ea2eb

@hulin32 hulin32 changed the title Fix/3253: add sourcemap file to chunk.auxiliaryFiles fix: add sourcemap file to chunk.auxiliaryFiles Apr 6, 2024
@github-actions github-actions bot added the release: bug fix release: bug related release(mr only) label Apr 6, 2024
@SyMind
Copy link
Member

SyMind commented Apr 8, 2024

Please create a unit test within the directory packages/rspack/tests/cases/chunks.

@web-infra-dev web-infra-dev deleted a comment from rspack-bot Apr 15, 2024
@SyMind
Copy link
Member

SyMind commented Apr 15, 2024

!bench

@rspack-bot
Copy link

rspack-bot commented Apr 15, 2024

📝 Benchmark detail: Open

Name Base (2024-04-15 73a9832) Current Change
10000_development-mode + exec 2.67 s ± 33 ms 2.66 s ± 30 ms -0.43 %
10000_development-mode_hmr + exec 685 ms ± 12 ms 686 ms ± 6.7 ms +0.05 %
10000_production-mode + exec 2.5 s ± 30 ms 2.49 s ± 23 ms -0.66 %
arco-pro_development-mode + exec 2.53 s ± 93 ms 2.49 s ± 94 ms -1.56 %
arco-pro_development-mode_hmr + exec 430 ms ± 4.5 ms 430 ms ± 3.4 ms +0.07 %
arco-pro_development-mode_hmr_intercept-plugin + exec 442 ms ± 2.2 ms 442 ms ± 3.9 ms +0.04 %
arco-pro_development-mode_intercept-plugin + exec 3.27 s ± 75 ms 3.28 s ± 54 ms +0.47 %
arco-pro_production-mode + exec 4 s ± 95 ms 4.03 s ± 75 ms +0.72 %
arco-pro_production-mode_intercept-plugin + exec 4.76 s ± 59 ms 4.79 s ± 113 ms +0.63 %
threejs_development-mode_10x + exec 2.05 s ± 18 ms 2.04 s ± 21 ms -0.54 %
threejs_development-mode_10x_hmr + exec 768 ms ± 20 ms 769 ms ± 10 ms +0.18 %
threejs_production-mode_10x + exec 5.18 s ± 38 ms 5.16 s ± 21 ms -0.38 %

@SyMind
Copy link
Member

SyMind commented Apr 15, 2024

I ran the performance test and it looks like there are no issues. Then solve the other TODOs and we can be merged.

@hulin32
Copy link
Contributor Author

hulin32 commented Apr 16, 2024

Please create a unit test within the directory packages/rspack/tests/cases/chunks.

@SyMind dumb question, I wrote the test, but how can I run the test in this folder to test it on my local ?
I am trying with this from doc:

# In root path
./x build -a
./x test js

but not work, thanks

@SyMind
Copy link
Member

SyMind commented Apr 17, 2024

@hulin32 The document is outdated. Please execute the pnpm run test command on packages/rspack now to perform the test. We will update the document later.

@hulin32
Copy link
Contributor Author

hulin32 commented Apr 17, 2024

@SyMind Hey, sorry for bother you, just for local test right now, I am putting below test codes in Errors.test.js file, and I want to use imported webpack to compile the file, then check the stats to see if auxiliaryFiles are good, but seems it isn't using the rspack from source codes, I put a print log println! in process_assets method which from crates/rspack_plugin_devtool/src/lib.rs, and ran cargo build && pnpm run build:cli:debug in root to compile it, then go to packages/rspack to run pnpm run test tests/Errors.test.js, but the println! log is not shown, so for me seems its not using the local one, any suggestions? thx a lot

it("should add source map file to auxiliaryFiles", async () => {
const stats = await new Promise((resolve, reject) => {
  const compiler = webpack({
    entry: {
		main: path.resolve(__dirname, 'cases/chunks/auxiliary-files/one.js')
	},
    devtool: 'source-map',
  });
  try {
    compiler.run((bailedError, stats) => {
      if (bailedError) {
        return reject(bailedError);
      }
      compiler.close(closeError => {
        if (closeError) {
          return reject(closeError);
        }
        resolve(stats);
      });
    });
  } catch (err) {
    // capture sync thrown errors
    reject(err);
  }
});
console.log("statssss", stats.toJson({ errorDetails: false }));
});

@SyMind
Copy link
Member

SyMind commented Apr 18, 2024

@hulin32 It's possible that stats.errors contains some error messages that could help diagnose the problem.

@SyMind
Copy link
Member

SyMind commented Apr 18, 2024

This is our current issue. Contributing a test case is quite difficult, so I will add the relevant unit tests.

@SyMind SyMind merged commit b0bc32b into web-infra-dev:main Apr 18, 2024
29 checks passed
@hulin32
Copy link
Contributor Author

hulin32 commented Apr 18, 2024

@SyMind awesome 👍, thanks for your help, also learn something here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release: bug fix release: bug related release(mr only)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants