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

Integration in Angular failing to import flattenChildren() #914

Closed
wlievens opened this issue Dec 30, 2021 · 28 comments
Closed

Integration in Angular failing to import flattenChildren() #914

wlievens opened this issue Dec 30, 2021 · 28 comments

Comments

@wlievens
Copy link

wlievens commented Dec 30, 2021

Describe the bug

I'm attempting to integrate the component in an Angular application.
This is partially successful, I made an Angular wrapper component where I instantiate the h5web component as follows:

ReactDOM.render(
  React.createElement(H5GroveProvider, {
    url: url,
    filepath: '',
    children: React.createElement(App, {
      startFullscreen: false
    })
  }),
  document.getElementById(this.rootId)
);

My backend is h5grove compatible at the URL where I am testing it. I can browse the HDF5 tree just fine and can see metadata, attributes, and scalar values.

However, when I click a multidimensional dataset node, I get the following error:

react_keyed_flatten_children__WEBPACK_IMPORTED_MODULE_5__ is not a function

I traced this to the following line in the Toolbar function/class and checked with some console prints, and it indeed isn't a function ... it's the react-keyed-flatten-children module object being referred at the following line:

const allChildren = flattenChildren(children).filter(isReactElement);

If I replace that line with:

const allChildren = flattenChildren.default(children).filter(isReactElement);

everything works fine! So somehow it is failing to recognise the default export on flattenChildren when I integrate the component in Angular.

Now, I don't know much about TypeScript and JavaScript's import magic, so I tried various combinations of setting the tsconfig values for esModuleInterop, allowSyntheticDefaultImports, isolatedModules and so forth but to no avail.

To Reproduce

I'm not sure anyone is interested in debugging my Angular setup since this is a React component so I was hoping for some wisdom from above regarding the peculiarities of TypeScript and/or JavaScript's import behaviour. If that leads us nowhere I can always try setting up a clean Angular sample app.

Expected behaviour

No errors on a trivial function import.

Screenshots

image

Context

  • OS: Windows
  • Browser: Chrome
  • Version: 1.3.0
  • H5Web context: @h5web/lib, @h5web/app
@wlievens
Copy link
Author

wlievens commented Jan 3, 2022

This issue seems related, however I don't know what role webpack plays in the whole setup

webpack/webpack#12675

@axelboc
Copy link
Contributor

axelboc commented Jan 6, 2022

Hey, thanks for the report! This is very enigmatic. Fortunately, we've done a round of dependency upgrades a few weeks ago in #897, after the latest v1.3.0 release. So before going further, I'd recommend waiting for the next release to see if the error disappears. We'll try to do the release asap, hopefully next week.

@axelboc axelboc added the waiting Waiting for another task to be completed label Jan 6, 2022
@wlievens
Copy link
Author

wlievens commented Jan 6, 2022

Thanks for the feedback. To add some more information: I tried an alternate setup where I made a small pure react app (with create-react-app cli) setting up just h5web, and I'm getting exactly the same issue.

Hopefully your new release helps out!

Also - doing all this on Windows. Maybe that's part of the issue.

@wlievens
Copy link
Author

Hey, is there any information on the schedule for the upcoming release? Thanks in advance!

@axelboc
Copy link
Contributor

axelboc commented Jan 12, 2022

Should be this afternoon or tomorrow. 😉

@wlievens
Copy link
Author

Perfect, thanks

@loichuder
Copy link
Member

v1.4.0 landed this morning.

You can give it a try and tell us if it fixes your issue 🙂

@wlievens
Copy link
Author

Unfortunately, the error persists:

image

To confirm that I am using the latest version:

image

@axelboc
Copy link
Contributor

axelboc commented Jan 13, 2022

Hmm sorry to hear that. Do you use npm or yarn as your package manager? We use pnpm in our projects, which could explain why we haven't encountered this error so far.

@wlievens
Copy link
Author

I use plain NPM. Maybe you can try making a small project where you import just h5web in a plain NPM setup?

@axelboc
Copy link
Contributor

axelboc commented Jan 13, 2022

Yep, I'm on it 😉

Got stuck for a sec on trying to create a new CRA app with v4.0.3 (they've released v5 recently, which we haven't tested yet). This is the magic command, FYI: npx create-react-app --scripts-version 4.0.3 my-app

@axelboc
Copy link
Contributor

axelboc commented Jan 13, 2022

So here is what I've done and it seems to work (on Windows 10, with node@16.4.0 and npm@7.19.0):

$ npx create-react-app --scripts-version 4.0.3 my-app
$ cd my-app
$ npm install --save @h5web/app

In App.js:

import "@h5web/app/dist/style-lib.css";
import "@h5web/app/dist/style.css";

import React from "react";
import { App, MockProvider } from "@h5web/app";

function MyApp() {
  return (
    <div style={{ height: "100vh" }}>
      <MockProvider>
        <App />
      </MockProvider>
    </div>
  );
}

export default MyApp;
npm start

image

@wlievens
Copy link
Author

Alright I'll give that a try on my side as well.

Just to be sure: with the MockProvider, you also pass through the "Toolbar" function?
This line is the one causing misery:

const allChildren = flattenChildren(children).filter(isReactElement);

@wlievens
Copy link
Author

wlievens commented Jan 13, 2022

Your example works on my end. And if I replace the MockProvider with a H5GroveProvider pointing to my backend URL, it also works. Guess I have to find out where I'm doing things differently now in my other experiments...

@wlievens
Copy link
Author

Ah, some progress! Thanks to your hint I noticed that my experimental application (outside of the actual react project) was using react-scripts 5. Reverting to v4 fixes the issue.

I'm trying to force the same changes now on my main application but I haven't gotten it fixed just yet. Will keep you updated.

@axelboc
Copy link
Contributor

axelboc commented Jan 13, 2022

Yeah, Toolbar is a generic component used by each visualization's toolbar (the bar below the breadcrumbs). The "scalar" and "raw" visualizations are the only ones without a toolbar, which is why the issue doesn't appear when you open a scalar dataset.

Given what we've excluded so far, I'd be willing to bet that the issue comes from Angular's webpack config/version.

@wlievens
Copy link
Author

It does seem like Angular 13 imposes Webpack 5, and I agree with your analysis that the issue seems to be there.

What would be required to get h5web working with Webpack 4? I don't know how many Angular versions I'd have to go back to get to Webpack 4, I'll try to look into it ...

@axelboc
Copy link
Contributor

axelboc commented Jan 13, 2022

We're currently working on another round of upgrades, including CRA v5 for the demo, which includes webpack 5. That being said, if I understand the issue you linked to, it seems webpack 5 will uncover the issue above, not fix it, so we may have to take the problem to react-keyed-flatten-children.

EDIT Sorry, the problem is not quite the same, since react-keyed-flatten-children uses tsc, not webpack.

@wlievens
Copy link
Author

I fear I don't understand enough about how webpack and all the other "behind the scenes" mechanisms work to contribute something useful :-|

@axelboc
Copy link
Contributor

axelboc commented Jan 17, 2022

We've just released @h5web/app@1.4.1-beta.0, which you can install by running npm install @h5web/app@next. Let us know how you go. If it still fails, maybe the next step will be for you to provide us with a repro repository so we can have a play with it.

@wlievens
Copy link
Author

wlievens commented Jan 17, 2022

I'm getting this funky error when I build my application:

./node_modules/@h5web/app/dist/index.js:1:0 - Error: Module parse failed: 'import' and 'export' may appear only with 'sourceType: module' (1:0)
File was processed with these loaders:
 * ./node_modules/@angular-devkit/build-angular/src/babel/webpack-loader.js
You may need an additional loader to handle the result of these loaders.
> import _asyncToGenerator from "C:\\....\\asyncToGenerator.js";
| var __defProp = Object.defineProperty;
| var __defProps = Object.defineProperties;

I have no idea what it's about, I'll try and google my way out.

@axelboc
Copy link
Contributor

axelboc commented Jan 17, 2022

Hmm yeah, okay. It's because I removed "type": "module" from the package.json of @h5web/app. I thought it might have helped, like it did for CRA v5, but clearly it didn't for your set-up. 😄

As mentioned, I think the way forward would be for you to set up a minimal reproduction repository that we can play with.

@wlievens
Copy link
Author

I created a minimal Angular project with a wrapper for h5web using the MockProvider.

It's the example project here:

https://github.com/wlievens/h5web-issue-914

Instructions in the README in the example dir should be enough.

@wlievens
Copy link
Author

Hey, have you had time to try and reproduce the issue?

@axelboc
Copy link
Contributor

axelboc commented Jan 20, 2022

I've managed to make it work, sorry for the wait. I've released the fix in a beta version: v1.4.1-beta.2. Let me know how you go.

After the PR above, #940, which did not fix the issue, I tried bundling react-keyed-flatten-children inside @h5web/lib as a last resort. I'm filtering it out of the externals configuration that we pass to Rollup when building the library: 5cf8015

@axelboc axelboc removed the waiting Waiting for another task to be completed label Jan 20, 2022
@axelboc
Copy link
Contributor

axelboc commented Jan 20, 2022

I'm still not convinced the problem comes from the react-keyed-flatten-children package. It provides a simple CommonJS bundle with a default function export: exports.default = flattenChildren -- nothing crazy -- so I feel like this should be supported by Angular/webpack 5.

Regardless, I've opened an issue grrowl/react-keyed-flatten-children#3 to suggest providing an ESM build.

@wlievens
Copy link
Author

I tested your new build and this works! I don't understand where the issue comes from either but I'm not very knowledgeable on how various kinds of bundles and imports in JS work, but I'm happy of course it works like this now.

@axelboc
Copy link
Contributor

axelboc commented Sep 12, 2023

Hi @wlievens, react-keyed-flatten-children now has an ESM bundle, so I've reverted the workaround that consisted in inlining it in the lib's output bundle (cf. #914).

It will go out in the next major release, along with the React 18 upgrade. Please do get back to us then, if the error comes back.

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

No branches or pull requests

3 participants