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

babel-loader rules ignores additional_paths that come from node_modules #208

Closed
BALOTIAS opened this issue Nov 15, 2022 · 5 comments · Fixed by #240
Closed

babel-loader rules ignores additional_paths that come from node_modules #208

BALOTIAS opened this issue Nov 15, 2022 · 5 comments · Fixed by #240
Assignees
Labels

Comments

@BALOTIAS
Copy link

Ruby version: 3.1.2
Rails version: 7.0.4
Shakapacker version: 6.5.4

Expected behavior:
Directories in webpacker.yml additional_paths should all be transpiled, even if they're inside node_modules. Otherwise third-party packages like let's say @hotwired/turbo-rails might not work for custom browserlist configurations. (I had to support Safari 14, and turbo needed to be transpiled because of this)

Actual behavior:
All directories inside node_modules are ignored by the babel-loader, even if they're set in additional_paths, like node_modules/@hotwired/turbo-rails


Here's my modified rule, that fixes this (refactor it as you will, also the imports):

The babel-loader rule includes all inclusions and excludes all node_modules by default, besides the ones that are in the inclusions.

const { resolve } = require('path')
const { realpathSync } = require('fs')
const { loaderMatches } = require('shakapacker/package/utils/helpers')

const {
  source_path: sourcePath,
  additional_paths: additionalPaths,
  webpack_loader: webpackLoader,
} = require('shakapacker/package/config')
const { isProduction } = require('shakapacker/package/env')

const inclusions = [sourcePath, ...additionalPaths].map(p => {
  try {
    return realpathSync(p)
  } catch (e) {
    return resolve(p)
  }
})

/**
 * Modified babel-loader config of `node_modules/shakapacker/package/rules/babel.js`.
 * We had to modify the loader to NOT exclude inclusions (this is in a way a bug of shakapaker).
 * When we exclude inclusions (in webpacker.yml @ additional_paths) that might be inside node_modules,
 * like node_modules/@hotwired/turbo-rails, they won't be transpiled!
 *
 * See:
 * - https://webpack.js.org/configuration/module/#ruleexclude 
 * - https://github.com/babel/babel-loader/issues/171 
 * - Original rule: node_modules/shakapacker/package/rules/babel.js
 *
 * @type {*}
 */
module.exports = loaderMatches(webpackLoader, 'babel', () => ({
  test: /\.(js|jsx|mjs|ts|tsx|coffee)?(\.erb)?$/,
  include: inclusions,
  exclude: [
    {
      // exclude all node_modules from running through babel-loader
      and: [resolve('node_modules')],
      // Do not exclude inclusions, as otherwise these won't be transpiled
      not: [...inclusions],
    },
  ],
  use: [
    {
      loader: require.resolve('babel-loader'),
      options: {
        cacheDirectory: true,
        cacheCompression: isProduction,
        compact: isProduction,
      },
    },
  ],
}))

Usage:

You can then use addional_paths for node_modules as well, example:

# webpacker.yml
additonal_paths = [
  'app/assets', 
  'engine/foo/app/assets',
  ...
  'node_modules/@hotwired/turbo',
  'node_modules/@hotwired/turbo-rails',
  'node_modules/@hotwired/stimulus',
  'node_modules/stimulus-use',
  ...
]
@BALOTIAS BALOTIAS added the bug label Nov 15, 2022
@vaukalak vaukalak self-assigned this Jan 3, 2023
@vaukalak
Copy link
Contributor

vaukalak commented Jan 3, 2023

Also if we should write the configuration in the babel.config.js file, if we want to transpile node_modules due to this issue: babel/babel-loader#149 (comment) . This is recommended configuration BTW.

@justin808
Copy link
Member

@BALOTIAS I think there should be a different configuration of paths that need transpiling inside of node_modules.

Is there any reason to overload the meaning of additional_paths?

@BALOTIAS
Copy link
Author

@justin808 I wasn't aware that additional_paths does more than simply adding files to webpack to look them up when resolving them. Maybe I misunderstood the docs: https://github.com/shakacode/shakapacker#additional-paths

Either way I'm happy for a different configuration of paths or to re-use the same, as long as I can somehow make the node modules transpile with my babel config. :)) Also it would make sense what @vaukalak mentioned.

vaukalak added a commit that referenced this issue Jan 25, 2023
vaukalak added a commit that referenced this issue Jan 25, 2023
vaukalak added a commit that referenced this issue Jan 25, 2023
@vaukalak
Copy link
Contributor

@BALOTIAS could you please try to set this as a dependency: #240 and lmk if this solves the issue?

@BALOTIAS
Copy link
Author

@vaukalak just tried it out, works! :D Might make sense to add a node_module path to the documentation example as well :))

vaukalak added a commit that referenced this issue Jan 26, 2023
ahangarha pushed a commit that referenced this issue Feb 11, 2023
justin808 pushed a commit that referenced this issue Feb 12, 2023
* fixed babel configuration for #208
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants