second-string / lineup-list

MIT License
7 stars 3 forks source link

Issues building locally due to private repo + outdated versions #13

Closed leccelecce closed 2 years ago

leccelecce commented 2 years ago

Tried to build with both Node 16.5 and then Node 13.7 on Windows 11

I had to delete package-lock.json as it seems a lot of packages resolve to plangrid.jfrog.io - a private repository? So I've lost build reproducibility.

I'm then getting the below during npm run build:

> tsc

src/routes/api.ts:81:60 - error TS2345: Argument of type 'string | string[] | ParsedQs | ParsedQs[]' is not assignable to parameter of type 'string'.
  Type 'string[]' is not assignable to type 'string'.

81             await spotifyHelper.getAccessTokenFromCallback(req.query.code, req.query.error);
                                                              ~~~~~~~~~~~~~~

src/routes/pages.ts:81:44 - error TS2345: Argument of type 'string | string[] | ParsedQs | ParsedQs[]' is not assignable to parameter of type 'string'.
  Type 'string[]' is not assignable to type 'string'.

81         const queryYear: number = parseInt(req.query.year, 10);
                                              ~~~~~~~~~~~~~~

Weirdly, when playing around with pinning versions in package.json I did briefly managed to fix this - but then in experimenting as to which module was causing the problem it came back and I've not been able to reproduce the fix. I tried pinning Typescript to 3.7.5 but no joy.

Any advice welcome - I'm quite new to Node so might be missing something obvious

Worth noting that there are quite a few vulnerabilities reported in the used versions during build

second-string commented 2 years ago

Haha that's so bad I'm so sorry. That's an old company repo, I probably built from NPM for the first time while traveling from work so it defaulted to the private artifactory URLs from the environment.

I didn't have an issue getting stuff built when I switched remote servers about a year ago though, so not sure where the issues are coming in. I'll take a look at this this week.

As for the vulns, yeah it's bad I've been really lazy. All it takes is me updating one at a time and verifying everything still works, but all of the dependencies that have vulnerabilities are internal dependencies of installed packages, so it's not as simple as a glance at changelogs for breaking changes

I'll devote some time this week to getting everything cleaned up in the repo and readme to make a reproducible build env easier. Ideally it'd just be a container but I don't really want to take the time to switch over to that right now

second-string commented 2 years ago

Also I'd recommend using the npm script targets for building and running to make the process simpler (might be something I need to update in the readme, not sure).

npm run build to check typescript compilation and run linting npm run dev to build and run with nodemon so the server restarts if you recompile (most useful if you have a plugin that runs tsc on save or something similar)

leccelecce commented 2 years ago

Glad I'm not going mad! The two errors above are easily solved with casts though unclear why they seem to come up intermittently for me. I just want to ensure I'm building something that's not a million miles to what's in prod...

second-string commented 2 years ago

Okay - I did my best with the vulnerability and updated a ton of packages but I couldn't get them all cleared out. npm audit fix --force doesn't seem to want to always update packages even though it's supposed to ignore pinned values and I don't really feel like spending more than the 45 minutes I already have on it.

I also got the type issues when I got everything upgraded. Pretty sure the issue was from the @types/express package newer version having a newer type for the query param field on a request. I just added casts since I only ever use normal query param behavior in URLs, aka strings.

I deleted my node_modules dir and the package-lock.json file, then ran npm i and everything worked fine. Don't use node 13.7 - that package is just the typescript @types package version. I use node v18 to build and run locally, but the remote linux box is an older LTS bistro and uses 14.18.1, so you should be good with anything above that. Let me know what you run into

leccelecce commented 2 years ago

Thanks for doing this so quickly.

I rebuilt successfully after deleting dist and node_modules, and can start the app, but am now getting these errors both on Node 14.7 and 18.4 when I actually submit a festival and year. Not sure why, can look into it my end unless you see anything obvious?

Getting spotify API token...
Getting spotify API token...
Getting spotify API token...
C:\Users\.\git\Lineup-List\dist\helpers.js:47
                        encodedBody = form_urlencoded_1.default(options.body);
                                                               ^

TypeError: form_urlencoded_1.default is not a function
    at Object.<anonymous> (C:\Users\.\git\Lineup-List\dist\helpers.js:47:64)
    at Generator.next (<anonymous>)
    at C:\Users\.\git\Lineup-List\dist\helpers.js:8:71
    at new Promise (<anonymous>)
    at __awaiter (C:\Users\.\git\Lineup-List\dist\helpers.js:4:12)
    at Object.instrumentCall (C:\Users\.\git\Lineup-List\dist\helpers.js:33:12)
    at C:\Users\.\git\Lineup-List\dist\spotify-helper.js:46:53
    at Generator.next (<anonymous>)
    at C:\Users\.\git\Lineup-List\dist\spotify-helper.js:27:71
    at new Promise (<anonymous>)
second-string commented 2 years ago

Yeah there was an error in the version of form-urlencoded that I updated to. I just pushed an new package.json with a newer version that fixes the broken typescript import. Running everything locally worked for me but I didn't go through any actions on the site like creating a new playlist, so there's a good chance something else is broken like this too

leccelecce commented 2 years ago

Thanks, I can now advance to the personalized playlist page. When I try to generate to Spotify, the Spotify API gives me an HTTP400 "INVALID_CLIENT: Invalid redirect URI" which I'm assuming is because it's trying to redirect back to my http://localhost:8080 and either it doesn't like http or the non-standard port.

Happy to consider this issue closed as the build is now succeeding, if I have any further problems will raise a new issue.

second-string commented 2 years ago

Sorry forgot to respond to this - the spotify oauth login redirect doesn't work with http. They mandate https so that step will never be able to work for http only.

second-string commented 2 years ago

In the readme: There might be some degraded functionality around the step of actually generating a playlist where we redirect to Spotify's login and then send you back. Create an issue if you find this is the case

Could be clearer instead of just "might be degraded functionality"