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

Fixing frontend failing to start #1989

Closed

Conversation

Muritavo
Copy link

@Muritavo Muritavo commented Dec 7, 2023

  • Restored NextJS version to previous version to fix next.config.js not found
  • Using require to import @vercel/analytics/react because of pkg bug ES modules not supported vercel/pkg#1291
  • Including CJS dependencies into pkg filesystem

What does this pull request do? Explain your changes. (required)

The docker images are unable to start the NextJS application. I "fixed" (quotes because I couldn't find the core problem on the related libraries) 2 problems:

  • 1º: next.config.js could not be found, even though the file was there and the suggested import to fix it was the same file 🤔.
Captura de Tela 2023-12-06 às 20 48 49
  • 2º: After fixing the first problem, a second one appeared. It couldn't find @vercel/analytics.

Both problems could only be reproduced when packaging the using the "pkg" command. I think both are related to this unclosed bug on pkg repository. Maybe it's something related to their virtual file system. More research needed.

Specific updates (required)

  • 1º: Downgrading the NextJS back to the previous lock file version seems to resolve the issue.
  • 2º: I changed the "import" to "@vercel/analytics/react" to a "require". This makes it load the .cjs extension module. Because of how "pkg" decides which dependencies files to include, I had to include on package.json an explicit indication for it to copy the files from @vercel/analytics node_modules folder.

How did you test each of these updates (required)

Since I don't have the necessary knowledge to test docker images, I tested it manually only.

  • Firstly I assured that the error was occurring locally with the latest version
  • Found the culprit commit
  • Downgraded NextJS version (fixing the 1º problem)
  • Tested it locally after each command and found that the pkg command was related to the @vercel import error
  • Changed the import to require and tested it again, opening the NextJS application (fixing the 2º problem)
  • Only tested the NextJS application and login. The changes shouldn't require testing other features.

Does this pull request close any open issues?

Probably these:
#1985

Checklist

  • I have read the CONTRIBUTING document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.

- Restored NextJS version to previous version to fix next.config.js not found
- Using require to import @vercel/analytics/react because of pkg bug
vercel/pkg#1291
- Including CJS dependencies into pkg filesystem
@Muritavo Muritavo requested review from a team as code owners December 7, 2023 00:03
Copy link

vercel bot commented Dec 7, 2023

@Muritavo is attempting to deploy a commit to the Livepeer Team on Vercel.

A member of the Team first needs to authorize it.

@Muritavo
Copy link
Author

This pull request have been ignored and I'm closing it since the culprit commit has been reverted. :(

@Muritavo Muritavo closed this Dec 13, 2023
@adamsoffer
Copy link
Member

adamsoffer commented Dec 13, 2023

@Muritavo Thanks so much for this PR. I'm really sorry the Studio team never got eyes on it. We want to encourage OSS contributors to get involved in the codebase and will make sure future PRs get addressed within a week tops.

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

Successfully merging this pull request may close these issues.

None yet

2 participants