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

transformer: React fast refresh transform #3943

Closed
Boshen opened this issue Jun 27, 2024 · 2 comments · Fixed by #4587
Closed

transformer: React fast refresh transform #3943

Boshen opened this issue Jun 27, 2024 · 2 comments · Fixed by #4587
Assignees
Labels
A-transformer Area - Transformer / Transpiler

Comments

@Boshen Boshen added the A-transformer Area - Transformer / Transpiler label Jun 27, 2024
@Boshen Boshen added this to the Transformer Milestone 2 milestone Jun 27, 2024
@Dunqing Dunqing self-assigned this Jul 31, 2024
@Dunqing
Copy link
Member

Dunqing commented Jul 31, 2024

Based on internally discussed priorities for milestone2, I plan to start development of this plugin. It's not expected to take long.

Dunqing added a commit that referenced this issue Aug 15, 2024
close: #3943

## Further improvements

There is a double visit here. We need to collect all react hooks calling in `Function` and `ArrowFunctionExpression`. If we want to remove this implementation, we may wait for #4188.

https://github.com/oxc-project/oxc/blob/d797e595d286c613848b773c256bd43124ad1981/crates/oxc_transformer/src/react/refresh.rs#L744-L947

## Tests

All tests copy from https://github.com/facebook/react/blob/main/packages/react-refresh/src/__tests__/ReactFresh-test.js

There are still 4 tests that have not been passed

**1. refresh/can-handle-implicit-arrow-returns/input.jsx**

Related to #4767. transform correct, just output doesn't match the expected output

**2. refresh/registers-identifiers-used-in-jsx-at-definition-site/input.jsx**
**3. refresh/registers-identifiers-used-in-react-create-element-at-definition-site/input.jsx**

Blocked by #4746

**4. refresh/supports-typescript-namespace-syntax/input.tsx**

oxc transforms ts to js first, so probably we can ignore this case. If we really want to pass this test, we also need to turn off `TypeScript` plugin.

## What's next?

### Options:

1. Support transform `refresh_reg` and `refresh_sig` options to `MemberExpression`. Currently `import.meta.xxxx` still is an `Identifier`
2. Support `emit_full_signatures` option

### Other
NAPI, testing in `monitor-oxc`, etc..
Dunqing added a commit that referenced this issue Aug 15, 2024
close: #3943

## Further improvements

There is a double visit here. We need to collect all react hooks calling in `Function` and `ArrowFunctionExpression`. If we want to remove this implementation, we may wait for #4188.

https://github.com/oxc-project/oxc/blob/d797e595d286c613848b773c256bd43124ad1981/crates/oxc_transformer/src/react/refresh.rs#L744-L947

## Tests

All tests copy from https://github.com/facebook/react/blob/main/packages/react-refresh/src/__tests__/ReactFresh-test.js

There are still 4 tests that have not been passed

**1. refresh/can-handle-implicit-arrow-returns/input.jsx**

Related to #4767. transform correct, just output doesn't match the expected output

**2. refresh/registers-identifiers-used-in-jsx-at-definition-site/input.jsx**
**3. refresh/registers-identifiers-used-in-react-create-element-at-definition-site/input.jsx**

Blocked by #4746

**4. refresh/supports-typescript-namespace-syntax/input.tsx**

oxc transforms ts to js first, so probably we can ignore this case. If we really want to pass this test, we also need to turn off `TypeScript` plugin.

## What's next?

### Options:

1. Support transform `refresh_reg` and `refresh_sig` options to `MemberExpression`. Currently `import.meta.xxxx` still is an `Identifier`
2. Support `emit_full_signatures` option

### Other
NAPI, testing in `monitor-oxc`, etc..
@Boshen
Copy link
Member Author

Boshen commented Aug 18, 2024

There are a few remaining issues but we'll close this issue for house keeping.

@Boshen Boshen closed this as completed Aug 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-transformer Area - Transformer / Transpiler
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants