Skip to content

feat: support for Vue3 JSX HMR#2018

Merged
chenjiahan merged 26 commits into
web-infra-dev:mainfrom
liyincode:feat/vue-jsx-hmr
Apr 10, 2024
Merged

feat: support for Vue3 JSX HMR#2018
chenjiahan merged 26 commits into
web-infra-dev:mainfrom
liyincode:feat/vue-jsx-hmr

Conversation

@liyincode

Copy link
Copy Markdown
Contributor

Summary

support vue3 jsx hmr

Related Links

issue #153

Checklist

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

@CLAassistant

CLAassistant commented Apr 7, 2024

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@netlify

netlify Bot commented Apr 7, 2024

Copy link
Copy Markdown

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.

Comment thread packages/plugin-vue-jsx/package.json Outdated
Comment thread packages/plugin-vue-jsx/package.json Outdated
Comment thread e2e/cases/vue/jsx-hmr/index.test.ts Outdated
await rsbuild.close();
});

// // not pass

@chenjiahan chenjiahan Apr 7, 2024

Copy link
Copy Markdown
Member

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

Comment thread packages/plugin-vue-jsx/src/index.ts Outdated
@chenjiahan chenjiahan changed the title Support Vue3 JSX HMR feat: support for Vue3 JSX HMR Apr 7, 2024
@chenjiahan

Copy link
Copy Markdown
Member

There are some conflicts need to be resolved 😄

image

@liyincode

Copy link
Copy Markdown
Contributor Author

There are some conflicts need to be resolved 😄

image

already resolved

@chenjiahan

Copy link
Copy Markdown
Member

Please fix the CI~

@liyincode

Copy link
Copy Markdown
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

9aoy commented Apr 8, 2024

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

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

@liyincode

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

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

@liyincode

liyincode commented Apr 10, 2024

Copy link
Copy Markdown
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

@liyincode

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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