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

Support npm link development workflow #1107

Closed
acailly opened this issue Nov 28, 2016 · 21 comments
Closed

Support npm link development workflow #1107

acailly opened this issue Nov 28, 2016 · 21 comments

Comments

@acailly
Copy link

acailly commented Nov 28, 2016

Hi,

I managed to add a shared project ("sharedApp") to my app made with create-react-app ("myApp") with npm link on Windows.

sharedApp contains only code and is not bundled, I wanted to be able to edit it and then myApp refresh automatically.

Here how I did, I found it a bit hacky and I am open for better solutions 😉

The start

I had the folder of sharedApp code in paths.appSrc:

appSrc: [resolveApp('src'), resolveApp('node_modules/shared-hybrid/src')],

First issue

Error in ***/shared-hybrid/src/components/common/Toast.js
Module parse failed: ***\shared-hybrid\src\components\common\Toast.js Unexpected token (15:2)
You may need an appropriate loader to handle this file type.
SyntaxError: Unexpected token (15:2)
 @ ./src/components/ObservationPage/ObservationPage.js 42:13-65

I replace in the paths:

appSrc: [resolveApp('src'), resolveApp('node_modules/shared-hybrid/src')],

by

appSrc: [resolveApp('src'), fs.realpathSync(resolveApp('node_modules/shared-hybrid/src'))],

Second issue

Nearly the same kind but not exactly

Error in ***/shared-hybrid/src/components/common/SelectServerDialog.js
Syntax error: ***/shared-hybrid/src/components/common/SelectServerDialog.js: Unexpected token (20:6)

  18 |
  19 |  const dialogActions = [
> 20 |       <FlatButton
     |       ^
  21 |         label="Ok"
  22 |         primary={true}
  23 |         keyboardFocused={true}

 @ ./src/components/HomePage/HomePage.js 54:26-91

I add the following code in the query of babel loader:

        query: {
          presets: [
            'react-app'
          ].map(dep => require.resolve(`babel-preset-${dep}`))
        }

Third issue

Error in ***/shared-hybrid/src/redux/reducers/toast.js
Module not found: 'babel' in ***\shared-hybrid\src\redux\reducers

 @ ***/shared-hybrid/src/redux/reducers/toast.js 12:15-36

Now I set:

resolve: {fallback: paths.appNodeModules}
resolveLoader: {fallback: paths.appNodeModules},

Final issue

It works! ... except I commented this section:

preLoaders: [
      {
        test: /\.(js|jsx)$/,
        loader: 'eslint',
        include: paths.appSrc,
      }
    ],

and when I suppress the comments, I get the following errors:

Error in ***/shared-hybrid/src/redux/actions/toast.js

***\shared-hybrid\src\redux\actions\toast.js
  1:1  error  Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

? 1 problem (1 error, 0 warnings)

To fix that, I add in shared-hybrid/package.json:

  "eslintConfig": {
    "extends": "react-app"
  }

And it works!

Edit : Oops, the tests are not working, jest doesn't transform the shared code and raise an error on import

@gaearon
Copy link
Contributor

gaearon commented Nov 28, 2016

Could you add a symlink inside src instead? Sounds like that would solve your problem better.
See #1065 for details.

@acailly
Copy link
Author

acailly commented Nov 28, 2016

Does it mean create a directory node_modules directory in src and do the npm link ..\..\shared-hybrid there ?
I have the same issues than before and the fixes don't work anymore.

@gaearon
Copy link
Contributor

gaearon commented Nov 28, 2016

I’ll try to get a look at this after we cut the next release. It won’t be a priority for now but it’s something we’d want to have a solution for.

@gaearon gaearon added this to the 1.0.0 milestone Dec 5, 2016
@gaearon gaearon changed the title Adding a shared project with npm link Support npm link development workflow Dec 5, 2016
@jperl
Copy link

jperl commented Dec 8, 2016

I have a project based off of lerna.

/packages/site-one # A create-react-app site
/packages/site-two # A create-react-app site
/packages/ui # Re-used react components and a react-storyboard

/packages/ui depends on react and is symlinked to /packages/site-one/node_modules and /packages/site-two/node_modules with lerna bootstrap.

To get around the issue Only a ReactOwner can have refs that happens because there are two reacts, we do this in the project we are working on:

cd packages/site-one/node_modules/react && npm link
cd ../../../ui && rm -rf node_modules/react && npm link react

@chiefGui
Copy link

I found a monster workaround to deal with local packages and it might help someone else: I have two projects that share some common code. Let's call them X and Y and the shared stuff is the ui.

So, I create the component I desire on X or Y, then I migrate it to ui (with all the tests passing—this is important to keep consistency!), yarn build to generate an output and only then I consume it through import {Component} from 'ui'.

This option was chosen to avoid big effort on making babel transpile node_modules/ui/components/ correctly. X and Y also do have some aliases that were conflicting with those on ui, which as you can imagine, is a pain in the ass to manage.

@tuchk4
Copy link
Contributor

tuchk4 commented Jan 6, 2017

I've created PR that is possible solution for npm-link workflow #1356

@gaearon
Copy link
Contributor

gaearon commented Feb 24, 2017

Fixed by #1356. Expected to be out in 0.9.1 when we release it.

@gaearon gaearon closed this as completed Feb 24, 2017
@gaearon gaearon reopened this Feb 24, 2017
@gaearon
Copy link
Contributor

gaearon commented Feb 24, 2017

Actually no, #1365 only fixes the workflow of linking custom react-scripts.
I don't think it solves this issue which is about developing libraries/components.

@kitze
Copy link

kitze commented Mar 23, 2017

@gaearon any temp workaround for this?

@chiefGui
Copy link

@kitze #1107 (comment) still working for me. 💃

@nikoloza
Copy link

nikoloza commented Mar 26, 2017

+1, I'm having similar issue as in here and here #1699, but also from non-linked npm module.

node_modules/some-package/index.js:

export default {}

src/index.js

import some from 'some-package'

error:

Uncaught SyntaxError: Unexpected token export
(in node_modules/some-package/index.js)

This happens not only on npm packages, but everywhere outside src.

I'm aware that src/node_modules fixes that, but can't make it clear how do you manage having node_modules in subfolder without lerna or something.

@gaearon
Copy link
Contributor

gaearon commented Mar 28, 2017

ES6 exports will not work inside node_modules untranspiled. We intentionally don't transform node_modules to prevent odd bugs and slow builds.

When we switch a stable version to Webpack 2 it will work but you should still transpile your libraries.

@nikoloza
Copy link

nikoloza commented Mar 28, 2017

@gaearon yes for production I can, but for dev (if you have it linked), it's pain to transpile it on every save.

@canercandan
Copy link

canercandan commented Jul 16, 2017

@chiefGui I tried your workaround with my components (let's say similiar to A and B using ui component). I yarn build the ui component and tried to import ui into A or B but the error remains the same SyntaxError: Unexpected token import.

I even tried to add the following babel preset in the ui package.json file:

  "babel": {
    "presets": [
      "react-app"
    ]
  },

I am also not very confortable of having a second node_modules into the src folder.

@chiefGui
Copy link

@canercandan The problem you have is simple: you're importing a lib that's not transpiled.

My bet would be to point your built script at package.json's entry property—otherwise you'll be trying to import a non-transpiled file that will lead to this error.

@temaivanoff
Copy link

Hi, I will speak only about the development

I have a project project "A", "B", "UI"

cd ./projects/UI
npm link

cd ./projects/A
npm link UI

cd ./projects/B
npm link UI

If I do npm start for project A or B, I get the following error
Unexpected token (4:2)
The reasons for this error are understandable. There is no BABEL processing
But if you do npm run eject for projects A and B and add the
./projects/A/config/webpack.config.dev.js
./projects/B/config/webpack.config.dev.js
following

  resolve: { symlinks: false } 

Then everything works as it should. Is it possible to influence this parameter without npm run eject?
Sorry for my English

@canercandan
Copy link

canercandan commented Jul 28, 2017 via email

@marcospassos
Copy link

marcospassos commented Oct 9, 2017

Any workaround for this that does not require ejecting or manually rebuilding all the dependencies whenever they change? Our use case is the same as #1107 (comment).

guilhermecvm added a commit to guilhermecvm/dolphino that referenced this issue Nov 28, 2017
problem: create-react-app does not support compiling files outside 'src'. Building draft-js*-plugin everytime it changes in development is not a good flow, so holding this for now
facebook/create-react-app#1107
@gaearon
Copy link
Contributor

gaearon commented Jan 21, 2018

The use case of sharing components between apps will be covered by #1333 (being implemented in #3741) so you can subscribe to them if you're interested.

However, the above solutions rely on using monorepos. This is because npm link behavior is fundamentally broken by design. I don't think we can truly fix it.

This thread dives into the problem in more details: webpack/webpack#985 (comment). Note there are simple solutions that seem right but have really bad pitfalls: webpack/webpack#985 (comment).

And even though webpack/webpack#985 (comment) takes care of the issue for webpack itself, we still can’t fix Node.js resolution mechanism now. So tests won’t work as expected. To me, this means that the whole approach is flawed and I recommend avoiding npm link in development.

I hope the monorepo approach in #1333 will solve most existing use cases for npm link.

@gaearon gaearon closed this as completed Jan 21, 2018
@gaearon gaearon removed this from the 100.0.0 milestone Jan 21, 2018
@gaearon gaearon reopened this Jan 21, 2018
@gaearon
Copy link
Contributor

gaearon commented Jan 21, 2018

Having read some discussions in nodejs/node#6537 and https://github.com/isaacs/node6-module-system-change I think we're actually already being inconsistent in how we handle symlinks.

Let's forget about the issue about compiling source code for a second. I feel like that can be solved by #1333 and thus is not the most important one here.

The important part here is that the resolution should match Node algorithm so that our webpack setup matches our test setup. I thought that was the case, but I was wrong.

Consider this structure:

my-comp
  index.js // already compiled
  package.json

my-app
  node_modules
    my-comp // symlink to my-comp folder

It turns out that if my-comp doesn't declare a dependency on react, it can find React in the browser builds but not in tests.

screen shot 2018-01-21 at 12 25 54

screen shot 2018-01-21 at 12 26 06

We introduced this regression in #1359. I think maybe we missed it because if my-comp does explicitly depend on React, the test passes (and in the browser we just get two Reacts).

@gaearon
Copy link
Contributor

gaearon commented Jan 21, 2018

I decided to spin that off into #3883 as a separate issue.

@gaearon gaearon closed this as completed Jan 21, 2018
@lock lock bot locked and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants