Skip to content

feat: support for Vue3 JSX HMR #2018

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

Merged
merged 26 commits into from
Apr 10, 2024

Conversation

liyincode
Copy link
Contributor

Summary

support vue3 jsx hmr

Related Links

issue #153

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

@CLAassistant
Copy link

CLAassistant commented Apr 7, 2024

CLA assistant check
All committers have signed the CLA.

Copy link

netlify bot commented Apr 7, 2024

Deploy Preview for rsbuild ready!

Name Link
🔨 Latest commit 840cbcb
🔍 Latest deploy log https://app.netlify.com/sites/rsbuild/deploys/6615f32f6cc6ac000874d600
😎 Deploy Preview https://deploy-preview-2018--rsbuild.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 72 (no change from production)
Accessibility: 97 (no change from production)
Best Practices: 100 (no change from production)
SEO: 92 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

await rsbuild.close();
});

// // not pass
Copy link
Member

@chenjiahan chenjiahan Apr 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can add a comment here:

// TODO: see https://github.com/web-infra-dev/rsbuild/issues/153

@chenjiahan chenjiahan changed the title Support Vue3 JSX HMR feat: support for Vue3 JSX HMR Apr 7, 2024
@chenjiahan
Copy link
Member

There are some conflicts need to be resolved 😄

image

@liyincode
Copy link
Contributor Author

There are some conflicts need to be resolved 😄

image

already resolved

@chenjiahan
Copy link
Member

Please fix the CI~

@liyincode
Copy link
Contributor Author

Hi, I forgot the run steps in the CONTRIBUTING.md before, I've followed the steps now and it should be fine.

But, After I merged the main branch, I ran pnpm run e2e, and found that one of the test cases didn't pass😅

image

@9aoy
Copy link
Contributor

9aoy commented Apr 8, 2024

Hi, I forgot the run steps in the CONTRIBUTING.md before, I've followed the steps now and it should be fine.

But, After I merged the main branch, I ran pnpm run e2e, and found that one of the test cases didn't pass😅

@liyincode did you run build again after you merge main branch?

@liyincode
Copy link
Contributor Author

Hi, I forgot the run steps in the CONTRIBUTING.md before, I've followed the steps now and it should be fine.
But, After I merged the main branch, I ran pnpm run e2e, and found that one of the test cases didn't pass😅

@liyincode did you run build again after you merge main branch?

oh, i didn't. I have now executed build and e2e test has passed👍

i exected pnpm run lint, is this passed?
image

@chenjiahan
Copy link
Member

i exected pnpm run lint, is this passed?

It seems that lint is passed. When executing ls-lint locally, the result is abnormal. You can ignore it and we will handle it.

@liyincode
Copy link
Contributor Author

Only windows didn't pass, I found two issues in the e2e test. The first one seems to need your help, and I will solve the second one on a Windows machine.
image
image

@chenjiahan
Copy link
Member

You can ignore the first decoratorBeforeExport error, it is expected 😄

@liyincode
Copy link
Contributor Author

Hi, I'm not sure what specifically causes the issue where e2e tests fail only on Windows platform.

Everything works fine on my Mac using a Windows 11 virtual machine, but the problem recurs on my home desktop with Windows 11. I suspect it might be related to the ARM vs x86 architecture or execution speed, but I'm unsure how to fix it.

Interestingly, when I test single e2e unit run npx playwright test cases/vue/jsx-hmr/index.test.ts -g "hmr: named export via specifier" within the e2e directory, everything works fine.

However, running all tests with npx playwright test cases/vue/jsx-hmr/index.test.ts reproduces the issue. I suspected it might be due to launching rsbuild run dev multiple times.

Surprisingly, by using a single rsbuild and page instance with test.describe, I found that the problem was resolved, and everything worked normally.

@chenjiahan
Copy link
Member

HMR related cases sometimes will fail in Windows, you can add a condition to skip that as a workaround:

image

There should be a more fundamental reason behind this, and we need to take time to investigate.

@liyincode
Copy link
Contributor Author

ok, i has disabled the hmr e2e test in windows, but there's another error I need to fix.🥲

@chenjiahan chenjiahan merged commit 298ec98 into web-infra-dev:main Apr 10, 2024
@chenjiahan
Copy link
Member

Thanks for your great work! 👍👍 It will be released in Rsbuild v0.6.0

@liyincode
Copy link
Contributor Author

liyincode commented Apr 10, 2024

I'm confused about the difference between these three test types

  • test
  • rspackOnlyTest
  • webpackOnlyTest

I'm using rspackOnlyTest to write e2e test cases, and using pnpm e2e:rspack -g vue pnpm e2e:webpack -g vue doesn't report an error

If I write an e2e test case using test, using pnpm e2e:rspack -g vue doesn't give me an error, but using pnpm e2e:webpack -g vue does!
image

@liyincode
Copy link
Contributor Author

I'm confused about the difference between these three test types

  • test
  • rspackOnlyTest
  • webpackOnlyTest

I'm using rspackOnlyTest to write e2e test cases, and using pnpm e2e:rspack -g vue pnpm e2e:webpack -g vue doesn't report an error

If I write an e2e test case using test, using pnpm e2e:rspack -g vue doesn't give me an error, but using pnpm e2e:webpack -g vue does! image

I now mix the use of rspackOnlyTest and test in this way, I don't know if there will be any problems.
image

@chenjiahan
Copy link
Member

That's ok. Some of the E2E tests in Rsbuild are run by both Rspack and Webpack at the same time. This is to check that the functionality of Rspack is correctly aligned with Webpack.

For the Vue related E2E cases, we only need to support Rspack, so we can use rspackOnlyTest in Vue E2E cases. rspackOnlyTest is a function based on the test method and can be mixed with test.

@liyincode
Copy link
Contributor Author

Thanks for your great work! 👍👍 It will be released in Rsbuild v0.6.0

Hahaha, thanks for the outstanding efforts of the rsbuild team, I'm very happy to be able to contribute a little.😃

@liyincode liyincode deleted the feat/vue-jsx-hmr branch April 10, 2024 02:42
@chenjiahan chenjiahan mentioned this pull request Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants