added support for including node_module folders #358
Conversation
| REACT_APP_TYPESCRIPT_NODE_MODULES_FOLDERS="@company-name" | ||
| ``` | ||
|
|
||
|
|
There was a problem hiding this comment.
I'd say this is a bit too complicated. Importing ts source files from node packages is more of an exceptional than regular behavior. Extending the include clause of the tsx? rule to cover the node_modules folder is more simple and does not hurt anyone not using it, since it is only applied in this very specific case.
There was a problem hiding this comment.
Are you sure? I feel like that would slow down a large project during dev builds/testing to process every package...
I think what may be best is us maintaining a fork for ourselves, and maybe I will write a medium article on what I changed for anyone else that wants to do the same; since it is a pretty limited use-case.
I don't want to decrease dev performance for all users of this package just to meet our niche requirement.
| @@ -90,11 +96,13 @@ module.exports = { | |||
| // These properties only exist before ejecting: | |||
| ownPath: resolveOwn('.'), | |||
| ownNodeModules: resolveOwn('node_modules'), // This is empty on npm 3 | |||
| typescriptModules: typescriptNodeModules, | |||
There was a problem hiding this comment.
See comment for README.md.
| @@ -172,7 +172,7 @@ module.exports = { | |||
| // Compile .tsx? | |||
| { | |||
| test: /\.(ts|tsx)$/, | |||
| include: paths.appSrc, | |||
| include: [paths.appSrc, ...(paths.typescriptModules || [])], | |||
There was a problem hiding this comment.
See comment for README.md.
| @@ -176,7 +176,7 @@ module.exports = { | |||
| // Compile .tsx? | |||
| { | |||
| test: /\.(ts|tsx)$/, | |||
| include: paths.appSrc, | |||
| include: [paths.appSrc, ...(paths.typescriptModules || [])], | |||
There was a problem hiding this comment.
See comment for README.md.
| @@ -41,7 +41,7 @@ module.exports = (resolve, rootDir, isEjecting) => { | |||
| ), | |||
| }, | |||
| transformIgnorePatterns: [ | |||
| '[/\\\\]node_modules[/\\\\].+\\.(js|jsx|mjs|ts|tsx)$', | |||
| '[/\\\\]node_modules[/\\\\](?!@zept).+\\.(js|jsx|mjs|ts|tsx)$', | |||
There was a problem hiding this comment.
Two issue hier:
- What exactly is the
@zept, and why is listed here? - If the
webpackrule fortsx?files covers thenode_modulesfolder, those files should not be listed on this ignore list.
There was a problem hiding this comment.
This is a mistake in that I updated the package for our internal use to also ensure tests processed the package as well. I feel including all packages in the transform would slow tests down too.
I am voting we close this PR as to niche.
I tested by changing the files directly in node_module folder until I was able to get my repository working properly.
I am hoping this is an acceptable extension of the project. If not, let me know and I will simply use my own repo for my project.
thanks!