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

Use SWC loader #29

Merged
merged 18 commits into from
Jan 28, 2022
Merged

Use SWC loader #29

merged 18 commits into from
Jan 28, 2022

Conversation

tomdracz
Copy link
Collaborator

@tomdracz tomdracz commented Jan 24, 2022

Investigates how can we use swc-loader as a replacement for babel-loader

So far, works fairly well but need to figure out configuration layer. Things like React support, TS, fast refresh where previously enabled by a check whether the relevant plugin is installed. With swc, they come built in but need to be enabled in the config so might need to use additional configuration layer or settle on some defaults

To explore/to decide/To do:

  • Configuration - inline with loader setup or using .swcrc file?
  • Configuring features - allow running React, TS, fast refresh by default or make user configurable? Is there a reason why we would not enable all of those?
  • Installing peer dependencies, no point in installing everything so might need to rejig how we install peer dependencies. For now could maybe leave swc out of peer dependencies and have manual install instructions
  • Explore how swc env matches up with @babel/preset-env - some options map one to one, but others are absent (works but browserslist is not processed, see Browserslist not being applied from .browserslistrc or package.json swc-project/swc#3365)
  • Untested, need to see how it behaves with .erb or .coffee files
  • Cleanup duplication and streamline swc loader rules
  • What tests would we cover this with?

@tomdracz
Copy link
Collaborator Author

Progress so far, tried hooking this up to https://github.com/shakacode/react_on_rails_tutorial_with_ssr_and_hmr_fast_refresh and got it to work nicely after some hurdles with server rendering.

Browserlists doesn't seem to be correctly applied unless specified directly in swc config. Opened swc-project/swc#3365 to in swc repo.

@justin808
Copy link
Member

@tomdracz tomdracz marked this pull request as ready for review January 26, 2022 16:53
package/rules/swc.js Outdated Show resolved Hide resolved
package/rules/index.js Outdated Show resolved Hide resolved
@tomdracz tomdracz changed the title WIP: Use SWC loader as an options WIP: Use SWC loader Jan 26, 2022
@justin808
Copy link
Member

@tomdracz let me know when you think this is ready!

@tomdracz tomdracz changed the title WIP: Use SWC loader Use SWC loader Jan 27, 2022
@tomdracz
Copy link
Collaborator Author

@justin808 Think this one is good to go.

I've added configuration layer using config/swc.config.js file. This is similar to how you would configure babel although bit more opinionated and specific to us. Still probably best of the options to allow customisation.

shakacode/react_on_rails_demo_ssr_hmr#30 also updated to show the migration to swc-loader

it will still be experimental until we get more usage (tested on the example app and our current prod app) but it's a start.

Will be good to add generator option to install that would set correct webpack_loader option, install the dependencies and maybe set some example config/swc.config.js but we can bring this in later when we move this out of "experimental" phase.

@justin808
Copy link
Member

@tomdracz I will check this out next week for sure, at the latest. Very exciting!

In terms of

What tests would we cover this with?

I think the gem should 2 big improvements to CI:

  1. A sample app, like /spec/dummy https://github.com/shakacode/react_on_rails/tree/master/spec/dummy
  2. CI steps that run rails new and the webpacker generator, and then a test, like those here in React on Rails

Take a look at https://github.com/shakacode/react_on_rails/blob/master/Rakefile and https://github.com/shakacode/react_on_rails/blob/master/.github/workflows/main.yml.

@justin808
Copy link
Member

@tomdracz Let me know if you'd like me to push out a pre-release 6.1 with support for this feature.

Copy link
Member

@justin808 justin808 left a comment

Choose a reason for hiding this comment

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

Looks GREAT!

Just some grammar and spelling is all that I can see without trying this out.

docs/using_swc_loader.md Outdated Show resolved Hide resolved
module.exports = customConfig
```

### Example: Enabling React Fast Refresh
Copy link
Member

Choose a reason for hiding this comment

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

We can either change this package or make a new one that makes this integration super easy.

https://github.com/shakacode/react-on-webpacker/

package/utils/helpers.js Show resolved Hide resolved
syntax: isTypescriptFile(filenameToProcess)
? 'typescript'
: 'ecmascript',
[isTypescriptFile(filenameToProcess) ? 'tsx' : 'jsx']:
Copy link
Member

Choose a reason for hiding this comment

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

Should we move React support back into Shakapacker? Seems that we should keep it the same for both babel and swc? However, now that I'm maintaining shakapacker, I don't mind having React support being supported by default.

rails/webpacker#3224

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@justin808 there's two parts to it - understanding jsx syntax and React transform. I think we can move the latter to a config but with the former relying on the filename it might need to stay.

I'll see how it handles non-React jsx files and adjust accordingly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@justin808 There's some weirdness with passing options to swc inline, for more details swc-project/swc#3374

This setup will work though regardless and I see no real reason not to support basic React config out of the box if we're not requiring any specific presets or plugins like @babel/preset-react. It "just" works so we might as well embrace it. We could drop the tsx/jsx setting here and move to the custom config but that will probably need to become a function that takes a filename to allow dynamic setting of jsx/tsx value (no idea why SWC just didn't settle for one setting 🤷 ).

Still, I've removed the transform setting from the default config and added section to the docs on how might one add settings to match any settings they have in preset-react if they wish to do so.

Copy link
Member

Choose a reason for hiding this comment

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

We'll iterate on this after the first release.

Maybe we can just be clear that SWC is experimental and the final API might change by the time 6.1.0 is released.

README.md Outdated Show resolved Hide resolved
@justin808
Copy link
Member

@tomdracz once I merge, I'll push a 6.0.0.beta.1 with this change.

Copy link
Member

@justin808 justin808 left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning up the grammar!

syntax: isTypescriptFile(filenameToProcess)
? 'typescript'
: 'ecmascript',
[isTypescriptFile(filenameToProcess) ? 'tsx' : 'jsx']:
Copy link
Member

Choose a reason for hiding this comment

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

We'll iterate on this after the first release.

Maybe we can just be clear that SWC is experimental and the final API might change by the time 6.1.0 is released.

@justin808 justin808 merged commit 40218f2 into shakacode:master Jan 28, 2022
@justin808
Copy link
Member

@tomdracz shakapacker-6.1.0.beta.0.gem and npm pushed.

Can you please update the changelog?

💥

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.

3 participants