Skip to content
This repository has been archived by the owner on Jan 26, 2019. It is now read-only.

Is using react *.tsx components from an npm package something that should work ? #348

Open
rluiten opened this issue Jun 26, 2018 · 9 comments

Comments

@rluiten
Copy link

rluiten commented Jun 26, 2018

Is this a bug report? No.

Running npm 5.10 node 8.94.
Totally clean start with one linked modules.
I am trying this out with dev mode npm start

Is putting react component typescript components into a npm package something that should work ?

I created a trivial local module with a single react component button TestButton.tsx it fails the solution compiles for the wrong reason.

My import import TestButton from 'lib/components/TestButton' from my local module lib is treated as a file-loader dependency. As a test I ejected to see if adding /\.(ts|tsx)$/, to exclude helped and it gets past being treated as a file asset.

          {
            // Exclude `js` files to keep "css" loader working as it injects
            // its runtime that would otherwise processed through "file" loader.
            // Also exclude `html` and `json` extensions so they get processed
            // by webpacks internal loaders.
            exclude: [/\.(js|jsx|mjs)$/, /\.html$/, /\.json$/],
            loader: require.resolve('file-loader'),
            options: {
              name: 'static/media/[name].[hash:8].[ext]',
            },
          },

After modify exclude to. exclude: [/\.(ts|tsx)$/,/\.(js|jsx|mjs)$/, /\.html$/, /\.json$/],
I then get another error

../lib/components/TestButton.tsx
Module parse failed: Unexpected token (3:7)
You may need an appropriate loader to handle this file type.
| import * as React from 'react'
| // stuff here
| export interface IProps {
|   onClick: () => void
| }
@DorianGrey
Copy link
Collaborator

To achieve this, you will need to change the include property of the rules for tsx? files to include the node_modules directory:

{
test: /\.(ts|tsx)$/,
include: paths.appSrc,
use: [

Note that this change has to be done for both dev and prod config.

@rluiten
Copy link
Author

rluiten commented Jun 27, 2018

@DorianGrey thanks for this, I will look into it asap and see how I go.
I had missed that exclude.

@rluiten
Copy link
Author

rluiten commented Jun 27, 2018

@DorianGrey I tried that in both files, it made no difference, I then added back the mode to file-loader i mention in first as well and went back to error in first. I tried removing the tsconfig.json exclude: 'node_modules' in my project folder but that didn't appear to help either.

@DorianGrey
Copy link
Collaborator

[...] I had missed that exclude.

I was pointing at an include, not an exclude - that's a major difference.

I tried that in both files [...]

that means what exactly?
Have you tried an include configuration like

include: [ paths.appSrc, /node_modules/ ]

in the test: /\.(ts|tsx)$/ rule?

@rluiten
Copy link
Author

rluiten commented Jun 28, 2018

My apologies I miss-typed, I also didn't make my context clear enough always a risk in written form. Let me try again.

1. My test process and what I am observing with unmodified react-scripts-ts v2.16.0

My test npm package called lib contains one component component/TestButton.tsx
My test create react app is called testapp and I install the lib package using npm install --save ../lib
so lib is a folder next my testapp folder, so using a local module.

In my testapp file App.tsx which is the default one from create react app typescript.
I import TestButton using import TestButton from 'lib/components/TestButton'.
And add it to the render() function.

I then run npm run build to produce production build as its a bit easier to see what is going on when it writes to disk.

The build works without any errors and outputs files to build folder inside testapp.

The TestButton.tsx file source file is output to the folder build/static/media/TestButton.983518d3.tsx and its just a text copy of the original source.

The TestButton.tsx component does not appear in the output file build/static/js/main.bbc1b693.js
I test this by having a simple unique string in the TestButton.tsx that cant found in the main bundle.

I check the bundle and can find

function(e,t,n){e.exports=n.p+"static/media/TestButton.983518d3.tsx"}

Referencing this static asset file.

2. I tried using paths.appNodeModules as follows.

I now modify react-create-ts v2.16.0 in webpack.config.prod.js

In the rule for test: /\.(ts|tsx)$/, for the loader loader: require.resolve('ts-loader').
I set the following include: [ paths.appSrc, paths.appNodeModules ], I found the paths.appNodeModules and it seemed it might be a better choice given the pattern in use throughout the config and script files.

I run npm run build and get the same result as case 1. same file name and everything

3. I tried using as you suggested /node_modules/

I now try your specific match path
In the rule for test: /\.(ts|tsx)$/, for the loader loader: require.resolve('ts-loader').
I set the following include: [ paths.appSrc, /node_modules/ ]

I run npm run build and get the same result as case 1 and 2 above same file name and everything

I doubt it matters but I am on Windows 10 operating system using node v8.11.3 and npm v5.10.0.

Thanks for your assistance, I am learning which is good.
Robin.

@DorianGrey
Copy link
Collaborator

Ok - if the pattern for node_modules does not match here, it means that the resolved file path of your lib is not in that folder. Yet it is obviously resolved, since the build does not fail. That's a bit surprising since a ModuleScopePlugin is used which prevents (or at least: should prevent) importing code outside of src oder node_modules.

I'm not that familiar with how local installations affect the resolution mechanism, esp. the path it returns.
Yet, if there is a lib folder outside of yours, you might try to add its path to the include declaration. For testing purposes, it should be sufficient to try something like path.resolve("..", "lib") and see if it gets picked up correctly. Or just try /lib\// (the trailing path separator is intended).

@rluiten
Copy link
Author

rluiten commented Jun 29, 2018

Ok your suggestions have let me make a bit more progress, if our lib folder physically exists instead of linked under node_modules things can work with your original suggested change.

I got rid of the default link behaviour of installing a local module by just copying the lib module into the node_modules folder as a test.

Using default react-scripts-ts v2.16.0 I still get that the TestButton is treated as an asset.

But now when I modify webpack.config.prod.js for the rule for ts-loader.
So /\.(ts|tsx)$/ rule now has include: [ paths.appSrc, paths.appNodeModules ] in it.

Now npm run build works fine and the typescript is pulled in and compiled properly.

Similary npm start now runs correctly.

Part of the reason for this asset behavior of imported typscript is the rule you pointed out, another part seems to be that the behaviour for installed local packages is to use links - on windows these links are Junctions, if the module is a link in node_modules it gets treated as an asset.

For a test I linked the module to another folder under src not under node_modules and imported it from that new location under src and it also got treated as an asset so not even involving the node_module folder in that case.

I have no idea if Windows link behaviour is different from linux I may have to run up a linux vm to try it, our dev platforms are all windows at the moment so even if it worked we cant just switch over either having other dependencies on windows.

Still not a great answer, of having a separated code module actually have as source path for editing be inside node_modules of another package I can see causing some bad side effects down the path.

I don't believe I would learned this much this quickly with out your help so far, thank you.
I will continue to try things and report what I can learn.

@rluiten
Copy link
Author

rluiten commented Jul 1, 2018

Some further information.

In the webpack config if I set resolve.symlinks: false , and also add paths.appNodeModules to the rule for ts-loader as you suggested then a local module installed as a linked module resolves correctly and npm run build and npm start both behave as desired. Both settings are required to get desired behaviour for me at the moment, this gets me away from needing to copy module into node_modules.

Documentation for the symlinks which appears to default to true.
https://webpack.js.org/configuration/resolve/#resolve-symlinks

It is not clear to me yet if this may have other negative side effects so will explore further.
I suspect if the item I am including is trying to import stuff on a relative path to itself that it may have issues depending on where the path ends up because the path report to webpack is now the symlinked location not where the actual module resides. So possibly just going above the top level folder of its own may present issues.

@rluiten
Copy link
Author

rluiten commented Jul 1, 2018

Got to testing by adding the external lib to the include path for ts-loader as you suggest.
Modifying webpack config to look as follows for ts-loader and just installing my external library as a link with npm install --save ../lib. I added the path.resolve('..', 'lib')

  // Compile .tsx?
  {
    test: /\.(ts|tsx)$/,
    include: [ paths.appSrc, path.resolve('..', 'lib') ],
    use: [
      {
        loader: require.resolve('ts-loader'),
      // the rest omitted.

No other modifications are required for build and start to work as desired.

Thanks for your input, I have learned a lot.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants