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 with package.json + sonar errors #269

Closed
cbtum opened this issue Feb 18, 2021 · 3 comments
Closed

Problem with package.json + sonar errors #269

cbtum opened this issue Feb 18, 2021 · 3 comments

Comments

@cbtum
Copy link

cbtum commented Feb 18, 2021

Thank you very much for the demo app, which really helps to understand how highcharts-angular works.

I noticed that highcharts-angular itself is missing in the dependencies of the package json and was only able to compile the project after adding it and running npm again.
"highcharts": "^9.0.0",
"highcharts-angular": "^2.10.0",
"highcharts-custom-events": "^3.0.10",

Besides there were some sonar warnings and errors (mainly quotemarks and a problem related to how rxjs is imported). As a beginner, sonar actually helps me a lot to understand how to do implement an angular app. Any chance that these warnings and errors get fixed?

@karolkolodziej
Copy link
Contributor

  1. Following the instruction from the documentation, npm install --> npm start I didn't have any issues with firing the demo app.
    What kind of errors/warnings did you get while compiling?

  2. Could you please explain what do you mean by sonar warnings and errors?

@cbtum
Copy link
Author

cbtum commented Feb 18, 2021

Thanks for your quick reply.

1st issue
After running npm install, I used ng serve to start the demo app (which leads to the following error:
Error: src/app/app.module.ts:5:39 - error TS2307: Cannot find module 'highcharts-angular' or its corresponding type declarations.

5 import { HighchartsChartModule } from "highcharts-angular";
~~~~~~~~~~~~~~~~~~~~
Clicking on http://localhost:4200/, I then run into an error and could not see anything.

Apparently, there are two alternatives to solve this problem:

  1. Use npm start, as proposed by you (which I simply did not see in the documentation)
  2. add "highcharts-angular": "^2.10.0", in the package.json between line 28 and 29, run npm install again and use ng serve.

The second option also adds highcharts-angular to the node_modules, which sounds reasonable.

2nd issue
SonarLint as well as TsLint are plugins that check my code and help me to detect issues and improve my code. Most of these warnings are only minor, such as e.g. pointing out that

  • quotemarks on the imports should always be './module' instead of "./module" (e.g. see the list of imports from the app.module.ts at the end of this post)
  • one should not import the entire rxjs library if only observables are used
  • the import $ from 'jquery' as well as the const data in lazy-loading-chart.components.ts (line 42) seem to be unused

I wouldn't dare to argue the way you do things is incorrect and should be improved. There might be very good reasons why the code is exactly as it is and should not be changed. Rather, as a beginner, I am happy to use these libraries and prefer to get rid of all these warnings in my own code, if possible. Usually, this only takes a couple of minutes to either fix these issues or supress the warnings. Since it might also help others and since I was asking for help regarding my ng serve issue anyway, I figured to add it to my question.

import { MapChartComponent } from "./map-chart/map-chart.component";
import { GanttChartComponent } from "./gantt-chart/gantt-chart.component";
import { LazyLoadingChartComponent } from './lazy-loading-chart/lazy-loading-chart.component';
import { LineTestComponent } from './tests/line-test/line-test.component';

@karolkolodziej
Copy link
Contributor

  1. Since it's not the recommended way to initiate the demo, we want treat that as a bug, but I think we should add a few words to documentation. I've created a new issue for that: Improve demo app description #270

  2. We also will improve these minor things which you described. Add some minor formatting changes in the demo app. #271

I split this issue into separate tickets so I'm going to close this one.

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

No branches or pull requests

2 participants