oklas / react-app-alias

:label: Alias and multiple src directory for craco or rewired create-react-app
MIT License
173 stars 18 forks source link

[FIX]Set configPaths options empty object by default #84

Closed AlanBlanchetNeovision closed 2 years ago

AlanBlanchetNeovision commented 2 years ago

The configPaths option can be empty by default and use the default attributes for tsconfig.json or jsconfig.json. Thus the options object should be allowed to be undefined so that we don't overload the config with an empty object if we don't need to define another path for ts/jsonconfig.

AlanBlanchetNeovision commented 2 years ago

Why add a PR ?

Because I got the following error :

    options.tsconfig || options.jsconfig
            ^
TypeError: Cannot read properties of undefined (reading 'tsconfig')
    at defaultOptions (/home/alan/dev/apv11_frontend/node_modules/react-app-alias/src/index.js:185:13)
    at aliasWebpack (/home/alan/dev/apv11_frontend/node_modules/react-app-alias/src/index.js:74:31)
    at Object.overrideWebpackConfig (/home/alan/dev/apv11_frontend/node_modules/react-app-alias/src/index.js:218:12)
    at overrideWebpack (/home/alan/dev/apv11_frontend/node_modules/@craco/craco/dist/lib/features/plugins.js:36:38)
    at /home/alan/dev/apv11_frontend/node_modules/@craco/craco/dist/lib/features/plugins.js:53:29
    at Array.forEach (<anonymous>)
    at applyWebpackConfigPlugins (/home/alan/dev/apv11_frontend/node_modules/@craco/craco/dist/lib/features/plugins.js:52:29)
    at mergeWebpackConfig (/home/alan/dev/apv11_frontend/node_modules/@craco/craco/dist/lib/features/webpack/merge-webpack-config.js:119:70)
    at overrideWebpackDev (/home/alan/dev/apv11_frontend/node_modules/@craco/craco/dist/lib/features/webpack/override.js:8:80)
    at /home/alan/dev/apv11_frontend/node_modules/@craco/craco/dist/scripts/start.js:21:39

Now able to do :

module.exports = {
  plugins: [
    {
      plugin: CracoAliasPlugin,
    },
  ]
}

Instead of :

module.exports = {
  plugins: [
    {
      plugin: CracoAliasPlugin,
      options: {}
    },
  ]
}
oklas commented 2 years ago

Hi! This is good. But your linting tool did rewrite the entire file. So I even do not see where is the change. Could you please disable linter and force push the changes again. So it will have only the change that actually implement the feature. I will be happy to merge. Also check if react-app-alias-ex need the same change. Thanks.

AlanBlanchetNeovision commented 2 years ago

@oklas Sorry about that. Had to disable my linter ^^. The react-app-alias-ex has the following variables imported :

const {
  aliasJest: aliasJestSafe,
  configFilePathSafe,
  readConfig,
  configPathsRaw,
  configPaths,
  defaultOptions,
  expandResolveAlias,
  expandPluginsScope,
} = require('react-app-alias')

It is dependant on react-app-alias-ex so everything should be okay.

oklas commented 2 years ago

Looks good. But it will be not noticed by rollout due to another commit convention (it uses angular commit convention). It requires commit message to be as "fix: set" instead of "[FIX]Set". Else it will not be considered as a reason to release new version.

AlanBlanchetNeovision commented 2 years ago

Should be good now @oklas

oklas commented 2 years ago

:tada: This PR is included in version react-app-alias-v2.2.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket: