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

feat: frontend js project type #135

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

feat: frontend js project type #135

wants to merge 20 commits into from

Conversation

Pehesi97
Copy link
Contributor

@Pehesi97 Pehesi97 commented Oct 20, 2023

To test this PR, go through the quick start guide with any angular, react or vue project as your demo app.

initium-cli should detect the project type by reading the package.json and checking for the "angular", "react" and "vue" keywords.

Closes #138

@Pehesi97 Pehesi97 requested a review from a team October 20, 2023 19:03
@Pehesi97 Pehesi97 self-assigned this Oct 20, 2023
@Pehesi97 Pehesi97 marked this pull request as draft October 20, 2023 19:03
@Pehesi97 Pehesi97 marked this pull request as ready for review October 24, 2023 21:32
@francardoso93
Copy link
Contributor

francardoso93 commented Oct 25, 2023

I was able to see this working locally through port forward 🎉
Had some issues using ingress, which were due to readiness failures. More details in other comments.


USER node

CMD npx http-server -p 3000 ./build
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Readiness probe fails, which is configured for port 8012 (No idea where that config is though)
  • Should we allow the default port to be configured?
  • Should we allow the static content folder "./build" to be configured? It could be "dist" or something else instead

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • it seems that the default port for a knative service is 8080, I'm trying that now
  • seems like a lot of effort, actually... but it's totally doable
  • same as the second point

We would need to add those as project flags, and not all of the project type we have right now would use them, so we would need to plan this a little bit.

I'd also like some feedback from @LucaLanziani

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK moving back to port 8080 worked.

example/frontend-js/dist/index.js Outdated Show resolved Hide resolved
src/services/project/project.go Outdated Show resolved Hide resolved
@francardoso93
Copy link
Contributor

francardoso93 commented Oct 25, 2023

Suggestion: Configure env var CI=true for Dockerfile, to avoid issues with React commands that freeze the CLI in watch mode. Eg: "test": "react-scripts test"

@@ -0,0 +1,14 @@
<!DOCTYPE html>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should add an actual basic react code here?

projectType = NodeProject
bytes, err := os.ReadFile(path.Join(directory, "package.json"))
var result map[string]any
json.Unmarshal(bytes, &result)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lack error validation

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.

Add frontend as one of the project type options
2 participants