Skip to content
This repository was archived by the owner on Jul 12, 2023. It is now read-only.

Fix register() calls inside lib/index.js - #11

Merged
j1elo merged 1 commit into
Kurento:masterfrom
PKSHATechnology:fix/require
Jun 26, 2020
Merged

Fix register() calls inside lib/index.js#11
j1elo merged 1 commit into
Kurento:masterfrom
PKSHATechnology:fix/require

Conversation

@whitphx

@whitphx whitphx commented Jun 26, 2020

Copy link
Copy Markdown
Contributor

What is the current behavior you want to change?

When this package is installed via npm and used via Webpack (and maybe other bundlers), the following error occurs at build time.

Error: Cannot find module 'kurento-client-core'

I think this is because require inside register which is dynamically called cannot be statically resolved by the bundler.

What is the new behavior provided by this change?

With this PR's change, the error above is resolved.
And this change should not affect anything as the new code is virtually same to the current one, because register called with a module name string without require like the current code ends up calling register with require internally(https://github.com/Kurento/kurento-client-js/blob/master/lib/register.js#L68).

How has this been tested?

I executed node test (npm test) and browser test (./node/bin/grunt -> open the file via browser).
In addition, I tested with Webpack project.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature / enhancement (non-breaking change which improves the project)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • My change requires a change to the documentation
  • My change requires a change in other repository

Checklist

  • I have read the Contribution Guidelines
  • I have added an explanation of what the changes do and why they should be included
  • I have written new tests for the changes, as applicable, and have successfully run them locally I have confirmed the results of the local tests are the same to the previous code.

@jenkinskurento

Copy link
Copy Markdown
Contributor

Hi there, thanks for your Pull Request!

A Kurento member needs to verify that this patch is reasonable to test. In case it is, they should write a comment with the phrase test this please. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by Kurento members will still work. Regular contributors can be whitelisted to skip this step.

… the target modules directly, but not modules names
@j1elo j1elo self-assigned this Jun 26, 2020
@j1elo
j1elo merged commit 7440e7e into Kurento:master Jun 26, 2020
@j1elo

j1elo commented Jun 26, 2020

Copy link
Copy Markdown
Member

Thanks!

@whitphx
whitphx deleted the fix/require branch June 26, 2020 11:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants