Unable to start a production build of concerto-ui in a CRA

[mttrbrts]

Replicated in https://github.com/shift-a-bit/concerto-ui-react-demo

Bug Report 🐛

Unable to start a production build of concerto-ui in a CRA. The development build works fine.
Reported by @shift-a-bit in AP Slack workspace.

Originally, I thought that this might be related to the new rollup pipeline, but replacing concerto-ui with [email protected] or [email protected] gives the same result.

Steps to Reproduce

git clone https://github.com/shift-a-bit/concerto-ui-react-demo
cd concerto-ui-react-demo
npm i
npm run build
serve -s build

[irmerk]

Not sure I understand this. We don't have build in our scripts and use lerna to handle this. @jeromesimeon you may have an idea?

[mttrbrts]

Not sure I understand this. We don't have build in our scripts and use lerna to handle this. @jeromesimeon you may have an idea?

The problem occurs when you build an app that includes one of our components as a dependency

[shift-a-bit]

Same issues is occurring if try to use MarkdownEditor as dependency. Both of the libraries are pointing to the modelmanager class in concerto-core library.
Error logs

   at new e (modelmanager.js:54)
    at new e (CommonMarkTransformer.js:58)
    at new e (CiceroMarkTransformer.js:52)
    at new e (SlateTransformer.js:31) 

[irmerk]

Took a quick look at this with @dselman and I have a suspicion it could be due to the way we export ConcertoFormWrapper here.

Though MarkdownEditor is exported in a seemingly normal way.

This may have to do with tweaking the rollup configuration?

[dselman]

Now looking at the dependencies...

We seem to have multiple versions of concerto-core in play, which might be an issue.

cicero-core depends on markdown-cicero 0.10.4 which depends on concerto-core 0.82.6

https://github.com/accordproject/cicero/blob/master/packages/cicero-core/package.json#L80

https://github.com/accordproject/markdown-transform/blob/v0.10.4/packages/markdown-cicero/package.json#L83

The rest of ui-concerto depends on concerto-core 0.82.7

[dselman]

@jeromesimeon can we release Cicero to align with concerto-core 0.82.7?

[jeromesimeon]

I think I'm able to reproduce this following @mttrbrts 's instructions.

You need serve. I used npm install -g serve

• Serve log:

bash-3.2$ serve -s build 
serve -s build 

   ┌───────────────────────────────────────────────┐
   │                                               │
   │   Serving!                                    │
   │                                               │
   │   - Local:            http://localhost:5000   │
   │   - On Your Network:  http://10.0.1.8:5000    │
   │                                               │
   │   Copied local address to clipboard!          │
   │                                               │
   └───────────────────────────────────────────────┘

In the browser I get an empty page:

[jeromesimeon]

Now looking at the dependencies...

We seem to have multiple versions of concerto-core in play, which might be an issue.

cicero-core depends on markdown-cicero 0.10.4 which depends on concerto-core 0.82.6

https://github.com/accordproject/cicero/blob/master/packages/cicero-core/package.json#L80

https://github.com/accordproject/markdown-transform/blob/v0.10.4/packages/markdown-cicero/package.json#L83

The rest of ui-concerto depends on concerto-core 0.82.7

I'm confused about how this relates to the issue. AFAICT there is no cicero involved in the demo here. Only Concerto and Concerto UI.

In my node_modules I only see:

  /Users/jeromesimeon/git/concerto-ui-react-demo/node_modules/@accordproject:
  drwxr-xr-x    16 jeromesimeon  staff    512 Jun 22 11:06 concerto-core
  drwxr-xr-x     5 jeromesimeon  staff    160 Jun 22 11:06 concerto-ui

The version of concerto-core is indeed 0.82.7 in there.

[DianaLease]

I can reproduce, both in the repo mentioned in the original issue and in a brand new create-react-app environment. I have tried with ui-markdown-editor, ui-contract-editor, and ui-concerto, and each produce the same error.

I'm still researching the problem, but I wanted to bring up this older similar (possibly the same?) issue I found in case it rings any bells (cc @jeromesimeon ): accordproject/concerto-ui#26

[jeromesimeon]

I can reproduce, both in the repo mentioned in the original issue and in a brand new create-react-app environment. I have tried with ui-markdown-editor, ui-contract-editor, and ui-concerto, and each produce the same error.

I'm still researching the problem, but I wanted to bring up this older similar (possibly the same?) issue I found in case it rings any bells (cc @jeromesimeon ): accordproject/concerto-ui#26

I had completely forgotten about this. Looking back at that other issue, I think it was addressed by moving away from CRA and using webpack instead. I don't think I ever figured out what the root of the problem is, but I admit CRA feels like a complete magic box to me...

[DianaLease]

The last version that does not have this error is
@accordproject/[email protected]. The error seems to have been introduced in @accordproject/[email protected] and has since been an issue in subsequent versions and packages.

This is particularly strange as it does not seem like anything changed between 0.82.8 and 0.82.9 besides the version numbers (accordproject/concerto-ui@6c786be). 🤔

[DianaLease]

Alright, I was having trouble following how the concerto-core-react and concerto-ui-core used to be updated, but now I see in the commit mentioned in my comment above, the concerto-ui-core dependency was bumped from 0.71.8 to 0.82.9, so there were other changes in that library. Particularly, that library's dependencies was updated from "composer-concerto": "^0.71.3" to "@accordproject/concerto-core": "^0.82.6" (accordproject/concerto-ui@36cc2c6#diff-08d95acff2c101079e3e5e797733ff0c).

This leads me to believe the issue may have been introduced in @accordproject/concerto-core, which would make sense based on the stack trace of the error. Still digging...

[DianaLease]

One last note before I leave this for the night. I think I see where @dselman was coming from with the differing versions - I read through another older issue (accordproject/concerto#47) around conflicting versions causing trouble for instanceof, and indeed the compiled code that is throwing this error uses instanceof (see below).

function _classCallCheck(instance, Constructor) {
  if (!(instance instanceof Constructor)) {
    throw new TypeError("Cannot call a class as a function");
  }
}

However, as @jeromesimeon already mentioned, the error is thrown even when the only version of concerto-core being used is 0.82.7, so I am not sure that is relevant here unless I am possibly missing something.

[jeromesimeon]

One last note before I leave this for the night. I think I see where @dselman was coming from with the differing versions - I read through another older issue (accordproject/concerto#47) around conflicting versions causing trouble for instanceof, and indeed the compiled code that is throwing this error uses instanceof (see below).

function _classCallCheck(instance, Constructor) {
  if (!(instance instanceof Constructor)) {
    throw new TypeError("Cannot call a class as a function");
  }
}

However, as @jeromesimeon already mentioned, the error is thrown even when the only version of concerto-core being used is 0.82.7, so I am not sure that is relevant here unless I am possibly missing something.

I think this is actually on to something. Between the old composer-concerto and the new concerto-core we changed the way instance of works so we could properly do m instance of ModelManager cross-copies of the same version of Concerto.

I think that might be why this fails. I think I can figure out a fix, but this will need republish of concerto-core.

[jeromesimeon]

A brute-force confirmation that the issue is with the overriding of instanceof in the new concerto classes. Those lines in e.g., the model manager:
https://github.com/accordproject/concerto/blob/f91da652e15485cd92db8576a4e0e60572a8542e/packages/concerto-core/lib/modelmanager.js#L709

Without those you can properly start the app:

[jeromesimeon]

I think this open issue in babel is related: babel/babel#4452

[dselman]

Do we ever actually call instanceof on the ModelManager?

[DianaLease]

@dselman When we call new ModelManager(), Babel runs this function:

function _classCallCheck(instance, Constructor) {
  if (!(instance instanceof Constructor)) {
    throw new TypeError("Cannot call a class as a function");
  }
}

[DianaLease]

Hi @shift-a-bit ! We aim to have this working with CRA in the future, but for now you can use this starter React app with a basic Webpack and Babel config to get started with using AP web components in a React app. The app includes an example of both Concerto Form and Markdown Editor. Note that it's using a new micropublish of these components - "@accordproject/ui-concerto": "0.93.2-20200626160144" and "@accordproject/ui-markdown-editor": "0.93.2-20200626160144" - as we had to make some changes to the configs inside the libraries themselves as well.

[shift-a-bit]

Hi @DianaLease ,Thanks a lot. I have checked the starter app. I can eject the configs from CRA for the time.

[rsilva-uw]

Having the same issue after building. When trying to reproduce it in a more basic project, I also had this issue with the start script. I've put it on a codesanbox ( even though it's not working in there, because of some process 😕 ) where you can zipit, yarn it and see the issue. Also it's important to register that I've managed to build a working solution by changing both the BABEL_ENV and NODE_ENV to development instead of production

[dselman]

I've reproduced this with a new App created using create react app. I've also verified that when we remove all our custom hasInstance methods from Concerto then the production build works.

    /**
     * Alternative instanceof that is reliable across different module instances
     * @see https://github.com/hyperledger/composer-concerto/issues/47
     *
     * @param {object} object - The object to test against
     * @returns {boolean} - True, if the object is an instance of a Property
     */
    static [Symbol.hasInstance](object){
        return typeof object !== 'undefined' && object !== null && Boolean(object._isProperty);
    }

My theory is that our override of hasInstance is too strict, so it fails the code that @DianaLease pointed to:
#110 (comment)

[dselman]

See this issue, which appears to be our case as well: parcel-bundler/parcel#2347

We are setting a flag inside the constructor, which we refer to in our hasInstance implementation - however Babel generates code that checks the instance before our _isXXXX flag has been set, which causes the instance of check to fail.