shakacode / shakapacker

Use Webpack to manage app-like JavaScript modules in Rails
MIT License
417 stars 91 forks source link

Shakapcker v8, Yarn Berry and PNP as nodelinker #482

Closed jeremybdk closed 4 months ago

jeremybdk commented 4 months ago

Hello, According to the latest removal for readme.md, it seems that shakapacker v8 should be compatible with yarn v2 and pnp as nodeLinker, but it seems that the default configuration is not working. Is there a way to get shakapacker working easily with yarn berry and pnp as nodeLinker ?

Thanks !

Expected behavior:

Out of the box support for Yarn Berry + pnp as nodelinker

Actual behavior:

The current install script will add "babel": { "presets": [ "./node_modules/shakapacker/package/babel/preset.js" ] }, which is not compatible with pnp as a nodelinker

Small, reproducible repo:

Create a new rails app, add a package.json with yarn 4.2.2, enable yarn berry, install yarn package then add shakapacker. The web server will not compile the JS.

Setup environment:

justin808 commented 4 months ago

We're investigating, @tomdracz @G-Rath.

G-Rath commented 4 months ago

I'll have a look later this week, but you should be able to use our rails-template as a reference on how you can do this

G-Rath commented 4 months ago

Ok so this is just a thing of Yarn PnP which makes sense in that context: configuring babel configs/presets via package.json involves it automagically requiring those files across the package boundary which is directly against Yarn PnP.

You need to use a babel.config.js in this case, which extends the Shakapacker babel config:

'use strict';

const defaultConfigFunc = require('shakapacker/package/babel/preset.js');

/** @type {import('@babel/core').ConfigFunction} */
const config = api => {
  const resultConfig = defaultConfigFunc(api);

  resultConfig.plugins = [...resultConfig.plugins];

  return resultConfig;
};

module.exports = config;

I'll do a PR to update the readme outlining that as a requirement if you're wanting to use PnP - the main other thing Shakapacker could do would be to switch to just using a babel.config.js instead of package.json, which I personally have no problem with doing 🤷

tomdracz commented 4 months ago

Thanks @G-Rath Merged! That might be enough in some setups to get this going.

We do have few places where we reference node_modules directly like https://github.com/shakacode/shakapacker/blob/57786174df5085f027f8db63ac30b1c64df54c20/package/environments/base.js#L70 https://github.com/shakacode/shakapacker/blob/57786174df5085f027f8db63ac30b1c64df54c20/package/rules/less.js#L16 https://github.com/shakacode/shakapacker/blob/57786174df5085f027f8db63ac30b1c64df54c20/package/rules/stylus.js#L17

And not sure if any further changes would be required here 🤔 Think the module resolution should automagically account for PNP modules and exclusions might be redundant with PNP though