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

flow types fail compilation in monorepo with CRA next.66cc7a90 #4517

Closed
bebbi opened this issue May 25, 2018 · 12 comments
Closed

flow types fail compilation in monorepo with CRA next.66cc7a90 #4517

bebbi opened this issue May 25, 2018 · 12 comments

Comments

@bebbi
Copy link
Contributor

bebbi commented May 25, 2018

Is this a bug report?

yes.

It's a project with flow annotations. Lines like these appear to trigger the issue:

import type { Props, ComponentState } from './MyComponent.type'

The error is:

Failed to compile.

./src/components/MyComponent.js
Module build failed: TypeError: Cannot read property 'type' of undefined
    at Array.forEach (<anonymous>)
    at Array.forEach (<anonymous>)

Moving the code outside the monorepo and yarn; yarn start: works.
Keep it inside and remove all flow annotations: works.

Did you try recovering your dependencies?

Yes.
In fact, adding a package.json declaration of

"nohoist": [
  "**/react-scripts", "**/react-scripts/**"
]

works around the issue.

Which terms did you search for in User Guide?

flow, monorepo, workspaces

Environment

Environment:
  OS:  macOS High Sierra 10.13.4
  Node:  8.9.1
  Yarn:  1.6.0
  npm:  6.0.1
  Watchman:  4.9.0
  Xcode:  Xcode 9.3.1 Build version 9E501
  Android Studio:  Not Found

Packages: (wanted => installed)

(paste the output of the command here)

Steps to Reproduce

  1. Create a CRA project from scratch
  2. Add flow annotations as above
  3. yarn; yarn start
  4. See it working
  5. Move the code into an existing monorepo with yarn workspaces enabled
  6. yarn; yarn start
  7. See it failing

Expected Behavior

Should just work.

Actual Behavior

Compilation failure per above error.

@Timer Timer added this to the 2.0.0 milestone May 25, 2018
@bugzpodder
Copy link

I cannot repro this. Can you provide a clean repo?
I also tried import { type Props } from "..." and it worked fine.

@Timer Timer modified the milestones: 2.0.x, 2.x Sep 26, 2018
@Timer Timer removed this from the 2.x milestone Nov 2, 2018
@stale
Copy link

stale bot commented Dec 2, 2018

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Dec 2, 2018
@apolkingg8
Copy link

Dear bot, it's still a issue.

@stale stale bot removed the stale label Dec 5, 2018
@bugzpodder
Copy link

Can you provide a repro?

@eduardomoroni
Copy link

eduardomoroni commented Dec 7, 2018

I'm facing the same issue with my current mono repo:
https://github.com/eduardomoroni/trading-card-manager
Folders structure

|_core
|_web (uses core)
|_mobile (uses core)

If you want to use this repo to reproduce:

image

I followed https://flow.org/en/docs/tools/create-react-app/.

Explaining better this case:
I'd like to share the module core between two apps. The module core is using flow annotations and type keywords. I'm installing this module via npm install ../core which make the apps create a symlink under its node_module folder. This module needs to be parsed by babel during run-time, the same way babel does with the files under src folder. What I think it's happening is that we're trying to run this module without transforming it.
I'm inclined to say that this problem is related to Babel and not CRA. The mobile project is using babel 6 and it's working fine
https://babeljs.io/docs/en/config-files#6x-vs-7x-babelrc-loading

@bugzpodder
Copy link

This should be import { type StateType } from '../types';
flowconfig

[libs]
../core/flow-typed
./flow-typed

[options]
module.name_mapper='^core\/(.*\)$' -> '<PROJECT_ROOT>/../core/src\1'

@eduardomoroni
Copy link

eduardomoroni commented Dec 8, 2018

Thanks @bugzpodder for you answer. I've tried you tip, no luck though.

When I tried to run flow, it got stuck.
image

When I try to run the App
image
Now it show Unexpected token, expected "," there's no comma on this line.

I also tried some variations of your suggestion, I thought that could be a typo on module.name_mapper='^core\/(.*\)$' -> '<PROJECT_ROOT>/../core/src\1', Based on the option docs.

Don't you think that this could be caused by how babel deals with Monorepos?

The changes you suggested was pushed on branch babel-error.

@bugzpodder
Copy link

Hey, sorry about that. I didn't pay attention to the details of the problem you described. So the issue isn't with flow typed, but it is because create-react-app is not stripping out flow-types from your symlinked node_modules/ when building your app.

So this is done on purpose as part of create-react-app configuration. The recommended way is to first run babel on your core module to strip out the flow types and then import it from web.
I have a working version here: https://github.com/bugzpodder/yarn-workspace-cra but it is using babel 7 and yarn workspaces.

So tooling (yarn workspaces/make) aside, here is what needs to happen:

  1. run babel on your core, generating dist/ from src/
  2. set package.json in your core to point to dist/
  3. link core to your node-modules
  4. set your flow-config to resolve import 'core/...' to 'core/src/...'

You maybe able to use a forked version of create-react-app, or otherwise modify the webpack config of react-scripts to achieve 1 - 3.

@eduardomoroni
Copy link

Thanks, @bugzpodder this worked great.
For what you said this behavior is part of the expected behavior of the library. So I think we can close this issue @bebbi.
Just out of curiosity, the reason why CRA doesn't allow transpile node_modules is to avoid unnecessary babel work?

@bugzpodder
Copy link

bugzpodder commented Dec 8, 2018

The reason is that create-react-app won't process libraries from using non-standard features such as flow and other js features and proposals that are not currently finalized, in order to avoid defragmentation of the js landscape.
What you are doing however is just sharing source code between different apps, so its a different use case that create-react-app isn't supporting right now but will in the future.

@stale
Copy link

stale bot commented Jan 12, 2019

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Jan 12, 2019
@stale
Copy link

stale bot commented Jan 18, 2019

This issue has been automatically closed because it has not had any recent activity. If you have a question or comment, please open a new issue.

@stale stale bot closed this as completed Jan 18, 2019
@lock lock bot locked and limited conversation to collaborators Jan 23, 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

5 participants