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

Merge is not a function at mergeH5PIntegration #2196

Closed
schmt5 opened this issue Apr 7, 2022 · 8 comments
Closed

Merge is not a function at mergeH5PIntegration #2196

schmt5 opened this issue Apr 7, 2022 · 8 comments

Comments

@schmt5
Copy link

schmt5 commented Apr 7, 2022

First of all, thanks a lot for your work.
In our implementation (based on h5p-rest-example-client) we get the following error:

h5p-utils.ts:36
Uncaught (in promise) TypeError: merge is not a function
    at mergeH5PIntegration (h5p-utils.ts:36:29)
    at H5PPlayerComponent.render (h5p-player.ts:389:9)

h5p-utils.js import the merge-function form deepmerge as ESM

// @lumieducation/h5p-webcomponents
// h5p-utils.js
import * as merge from 'deepmerge';

But in the deepmerge documentation is written:
https://www.npmjs.com/package/deepmerge#include

deepmerge exposes a CommonJS entry point:

const merge = require('deepmerge')
The ESM entry point was dropped due to a [Webpack bug](https://github.com/webpack/webpack/issues/6584).

Can you adjust the import? Or is this not the problem?
Another wierd thing I noticed is, that the error is from h5p-utils.ts but we just have a h5p-utils.js file. No src -folder is inside the h5p-webcomponents package.

@tuxes3
Copy link

tuxes3 commented Apr 7, 2022

changing the line to import merge from 'deepmerge'; works for me.

*edit: not all the times. still having the error...

@sr258
Copy link
Member

sr258 commented Apr 18, 2022

@schmt5 @tuxes3 Sorry about not answering earlier. I've turned off notifications for this repo as the renovate bot floods my inbox with pull requests. It's best to mention me, so I get a notification.

I think @tuxes3 solution should help. Changing to the outdated require is not an option. I can't reproduce your problem, however. Can you share a minimal example with which I can test a fix?

That the error comes from h5p-utils.ts is expected, as the library is transpiled from TypeScript. The src directory will be included in the next release.

@QuanTumli
Copy link

@sr258 Changing the import to import merge from 'deepmerge'; seems to work.

I will try to create a minimal example - we use ViteJS, and maybe there is a misconfiguration on our side during bundling.

But nevertheless I think the import from the deepmerge module should be changed. According to TehShrike/deepmerge#102 (comment) import deepmerge from 'deepmerge' should be the correct way.

Furthermore there was another library that needed to change its import to fix the same issue: getsentry/sentry-electron#403

@QuanTumli
Copy link

Hi @sr258 I could reproduce a minimal example with Vite.

I forked your repository, created a new branch issue/2196-vite-merge-is-not-a-function and added a new example folder /packages/ite-client-issue-2196. Follow the instructions inside the Readme to reproduce the issue: https://github.com/QuanTumli/H5P-Nodejs-library/tree/issue/2196-vite-merge-is-not-a-function/packages/vite-client-issue-2196

@jankapunkt
Copy link
Contributor

@sr258 I also get this error with our Meteor example when I play a content and then try to edit it.

@sr258
Copy link
Member

sr258 commented Jun 14, 2022

@QuanTumli @sr258 Sorry, about taking my time with this. I've changed the import and adapted the TypeScript configuration for it to work. Ultimately, I think this is caused by a difference in how the TypeScript compiler and Babel deal with imports if there is no default export (which is the case for deepmerge).

I've triggered the next release and the fix is included in it. If the CI pipelines run through (I'm a bit worried about GitHub rate limiting), the release should be out in a few minutes.

@sr258
Copy link
Member

sr258 commented Jun 14, 2022

The release is out on NPM (9.2.0). I would be happy about confirmation that this issue has been fixed.

@sr258 sr258 closed this as completed Jun 14, 2022
@QuanTumli
Copy link

@sr258 Yes it is working. Thank you for the fix! 💯

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

5 participants