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

Problem installing to npm project #655

Closed
raychien192 opened this issue Dec 24, 2022 · 20 comments · May be fixed by #695
Closed

Problem installing to npm project #655

raychien192 opened this issue Dec 24, 2022 · 20 comments · May be fixed by #695
Labels
enhancement New feature or request question Further information is requested

Comments

@raychien192
Copy link

Hello!
Please bear with me as I'm fairly new to npm.
I've tried incorporating this project into my npm project through "npm install musicbrainz-api" and followed the readme guide. However, I ran into all kinds of errors - they all seem to relate to Webpack 5.

Some of the errors I'm getting:
BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

WARNING in ./node_modules/musicbrainz-api/lib/xml/xml-isrc.js
Module Warning (from ./node_modules/source-map-loader/dist/cjs.js):
Failed to parse source map from ...

Uncaught ReferenceError: process is not defined
at ./node_modules/@szmarczak/http-timer/dist/source/index.js

So, at this point, I might resort to uninstalling it and referencing it for my own hand-crafted code - it would be easier that way!
Thanks

@Borewit
Copy link
Owner

Borewit commented Dec 24, 2022

This module is primary written for Node.je, I am not sure what it takes to get it up and running client side.

@Borewit Borewit added the question Further information is requested label Dec 25, 2022
@Haschikeks
Copy link
Collaborator

@raychien192 could you provide a reproduction repository?
or a general description of you setup?

@rome-legacy
Copy link

i have the same issue.

Angular CLI: 14.2.7
Node: 16.19.0
Package Manager: npm 8.19.3 
OS: linux x64

Angular: 14.2.8
... animations, common, compiler, compiler-cli, core, forms
... platform-browser, platform-browser-dynamic, router

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1402.7
@angular-devkit/build-angular   14.2.7
@angular-devkit/core            14.2.7
@angular-devkit/schematics      14.2.7
@angular/cli                    14.2.7
@schematics/angular             14.2.7
rxjs                            7.5.7
typescript                      4.7.4

i have a pretty new project with some unimportant components and services.

after adding musicbrainz-api with ng add musicbrainz-api
i get a long list of compilation errors:

build.log

thx

@Haschikeks
Copy link
Collaborator

Hey @rome-legacy
the main problem is, that this project is currently primarly intended to be used in a nodejs project and therefor not usable in the browser, or at least not without supplying webpack with polyfills.

@rome-legacy
Copy link

Hey,
thx for the reply. I'm not very familiar with the js/ts infrastructure. i thought the npm packages could be used, no matter if it is a nodejs app.
would it help to install all the dependencies, it is complaining about? or would it help to add webpack to the project? it wasn't planned, but i'm willing to try it :-)

it would be helpful to note this fact in the readme file.
in case i will be successful, maybe it will help others to use this library in other projects, but i guess, i will need your help while i'm trying it.

kind regards

@raychien192
Copy link
Author

@raychien192 could you provide a reproduction repository? or a general description of you setup?

Hello! My set up is actually quite specialized as I am building a React website through AWS Amplify.
I am still not too clear with its specifics - as I've found out later on, that Amplify bundles a lot of modules into it, with very specific versions of some of them. Installing any custom npm package into my environment that may trigger some dependencies being updated, would be unwise, and I have since then been more careful about it.

For now the direction of my development has taken me elsewhere and I'm not using Musicbrainz API as much. I will most likely write my own wrapper JS classes if I do, just to avoid breaking Amplify again. Thanks for your attention into this! Though if you have any tips on how to safely handle npm packages in an environment like mine, it will be much appreciated.

Thanks!

@Borewit
Copy link
Owner

Borewit commented Feb 7, 2023

if you have any tips on how to safely handle npm packages

Use git, you can always roll back your dependency configuration packages.json.
Checking in a your lock file gives even more strict control on locking your dependencies.
If required, delete node_modules folder and re-install npm install or yarn install again.

Do not backup node_modules, that is pointless.

@Haschikeks
Copy link
Collaborator

Hey, thx for the reply. I'm not very familiar with the js/ts infrastructure. i thought the npm packages could be used, no matter if it is a nodejs app.

@rome-legacy
NodeJS is a runtime used to run javascript on a server or computer environment. For example in a NodeJS app you don't have access to a DOM, but you have access to the filesystem. Now a days, we use NodeJS to build most of our web apps, like angular or react.

would it help to install all the dependencies, it is complaining about? or would it help to add webpack to the project? it wasn't planned, but i'm willing to try it :-)

Plainly installing all the dependencies could work, but I'm not sure it will.

In the end, this is something this package would need to change, so that it can be used in both the NodeJS, as well as the browser context.

@Borewit I might be able to look into getting musicbrainz running in the browser on the weekend. It should be possible to update the package without changing any of the API.

@Haschikeks Haschikeks added the enhancement New feature or request label Feb 7, 2023
@Borewit
Copy link
Owner

Borewit commented Feb 7, 2023

I thought the npm packages could be used, no matter if it is a nodejs app.

No that is not the case. Originally NPM was acutally setup for Node.js which introduced modulair dependencies. Also the web client envirnment started to use this mechanism, using bundlers like Webpack to convert modules to bundles.
Yet modules which rely on Node.js build in modules, certainly do not work out of the box in a browser.

Also the browser has API's which are not present on Node.js, so any combination is possible.

Certainly not every model works in every environment. This module is primary deisgned to work with Node.js. Looks like @Haschikeks is going to to an attempt to make it browser complient.

@rome-legacy
Copy link

I'm willing to help, somehow. But i do not have much time in the upcoming weeks. But on/off i will be available.

Thx @Haschikeks for trying. 👍

@rome-legacy
Copy link

Just out of curiosity... Why does the library need to be a node.js module? To me it sounds more like a client of an api. Is it the xml conversion that requires it to be a node module?

I took a brief look at the code and maybe it would be enough for me, if i extract the data interfaces to my project and make the calls on my own.

But i will definitely wait for Haschikeks experiment 😄

Other question: are there any xml schemas for the musicbrainz api?

@Borewit
Copy link
Owner

Borewit commented Feb 7, 2023

Why does the library need to be a node.js module?

It depends on got as the http client, which is written for Node.js.

Fundamental difference is that the HTTP API is totally different between your browser environment and Node.js.

So it not the case that we module developers have invented these difference.

Furthermore I can imagine you may run into connection restrictions from the browser, as will not connect you outside the website scope without Cross-Origin Resource Sharing (CORS).

Also the cookie handling is different, you browser should have good session handling in place. In Node.js there is nothing in place, you have rely on third party modules.

@rome-legacy
Copy link

Yeah, i've checked the dependencies and already thought that got the source of my problems is. Thx for clarification.

Regarding the cors problem: that should be the case also for serverside clients... Isn't it? If you do the calls from a different server?
But i expect a wildcard for cors of a public api... But you are right. I will check it with some postman requests.

For me it is important to have the data model so i can store it and (album/artist/song) and access it in a controlled way.

@Borewit
Copy link
Owner

Borewit commented Feb 7, 2023

MusicBrainz API documentation

On server side you can connect wherever you like. Browser code is typically downloaded from a website, hence much more restriction. That why in Node.js you simple access file system, in the browser you cannot.

@rome-legacy
Copy link

i did a quick experimental test by querying the api from my project (localhost) and it did work out just fine:

  // musicbrainz service
  public findByArtist(artistName: string): Observable<any> {
    const headers = new HttpHeaders({
      'User-Agent': 'staging.my-music-db',
      'Content-Type': 'application/json',
      'Accept': 'application/json'
    });

    return this.http.get(`https://musicbrainz.org/ws/2/recording/?query=artistname:${artistName}&limit=5`, {
      headers: headers
    }).pipe(
      take(1),
      catchError(err => {
        this.logger.error(err);
        return EMPTY;
      })
    );
  }
// some component
this.mb.findByArtist(artist?.name).subscribe(data => this.logger.debug(data));

@Haschikeks
Copy link
Collaborator

Hey,

quick feedback from me.
got is the primary concern here, but it can be substituted with something like ky-universal. But there might still be a small problem with cookies on the server side.

I took a brief look at the code and maybe it would be enough for me, if i extract the data interfaces to my project and make the calls on my own.

@rome-legacy if you only use the types, you may be able to use the package on client apps. But that depends on the bundlers ability to detect that you are only using the types
import type { IArtist } from "musicbrainz-api";
This is a way in typescript to tell the bundler or transpiler to only use that import for types. If you then have your own http calls you should be fine.


Something to note about using this package on the client side:

Keep in mind, that people might look into your source code and use your "credentials" for the musicbrainz api. Which could lead to your credentials beging rate limited or worst case even banned. I know that those credentials aren't real credentials, but it is still something to keep in mind. Have a look at their Rate Limiting for more information.

@Haschikeks
Copy link
Collaborator

Hey,

sorry for the late response. I've replaced "got" with ky-universal.
The branch is: 655-problem-installing-to-npm-project.
Using it in a nextjs environment I was able to use it on the client side, as well as the server side.
Sadly the test don't seem to like it.
@Borewit could you have a look at it?

@Borewit
Copy link
Owner

Borewit commented Mar 5, 2023

I will get back to this soon @Haschikeks, I have been busy with some other open source projects.
Feel free to raise a draft PR.

@Haschikeks
Copy link
Collaborator

Hey @raychien192, @rome-legacy,

here is some feedback after a discussion with Borewit, which you can have a look at in the pull request.

For now we will not be able to support running this package in the browser, we will update the readme file in the next couple of days to reflect that.

We are still in the process of finding a solution that works for both environments, browser & node.
If you want to use musicbrainz-api in the browser you need to do your own http calls, but can use the typings from this package.

@rome-legacy
Copy link

Ok.
Thx for the efforts guys. I think it is not very complex to implement some functions by myself and steal the data model interfaces.

Kind regards

@Borewit Borewit closed this as completed Nov 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
4 participants