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

initialized the enchancement of the client side developer experience #44

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

Conversation

zt4ff
Copy link
Contributor

@zt4ff zt4ff commented Oct 1, 2022

Hi @shubham1172

Since there's no specific direction to enhancing the client-side dev experience, I made the following as minimal as I could get.

  • webpack build with ts-loader for typescript and sass-loader for sass
  • move some event listeners from the HTML file into typescript
  • rewrote style.css into scss and have it included in the build process

Everything works fine , I look forward to your review and opinion.

@shubham1172 shubham1172 linked an issue Oct 3, 2022 that may be closed by this pull request
@shubham1172
Copy link
Owner

Thanks @zt4ff, I will take some time to review this PR since it brings in a couple of changes. Can we also add eslint/prettier, and create a GitHub pipeline to validate lint and build? If you don't want to add it to this PR, I can fork another issue and it can be worked on separately as well.

@zt4ff
Copy link
Contributor Author

zt4ff commented Oct 3, 2022

Thanks @shubham1172.

You can create a new issue for the eslint/prettier, Github pipeline in a new issues.
I will create a new branch to work on that.

I don't think they relate to the build process. I may be wrong though

@shubham1172
Copy link
Owner

Well, they are a part of the build IMO - builds must be validated on PRs, and their validity should also depend on the code (linting/pretty), but I won't increase the scope of this PR further and create a separate issue.

@zt4ff
Copy link
Contributor Author

zt4ff commented Oct 6, 2022

Alright, you can assign the new issue to me whenever you review this PR or should I just configure linting while this PR is open?

@shubham1172
Copy link
Owner

I will look at this PR over the weekend and also create a separate issue for the other things we discussed.

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
Co-authored-by: Shubham Sharma <[email protected]>
Copy link
Owner

@shubham1172 shubham1172 left a comment

Choose a reason for hiding this comment

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

Right now the root of this project is a python web server. Can we move the web part to a separate folder, web, that contains typescript code and configuration. The static folder can stay as is (outside web). This will help us clearly identify what does what.

static/index.html Outdated Show resolved Hide resolved
@zt4ff
Copy link
Contributor Author

zt4ff commented Oct 27, 2022

Hi @shubham1172
Sorry for the late response, I already resolved all the comments you made

@shubham1172
Copy link
Owner

Thanks for making those changes @zt4ff, this LGTM, I will try this out and merge the PR once I am done!

@shubham1172
Copy link
Owner

@zt4ff sorry for getting late to this (and it's ok if you don't have bandwidth to look into this), this PR does not seem to be working. After running the yarn commands, I start the web server and visit the index page where I get this error

Loading module from “http://127.0.0.1:8000/dist/index.js” was blocked because of a disallowed MIME type (“application/json”).

CONTRIBUTING.md Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@zt4ff
Copy link
Contributor Author

zt4ff commented Feb 13, 2023

@shubham1172

Hi, it's beena a while. Happy new year

I made new changes to fix the errors.

You can build the frontend with yarn build:dev and run python main.js to test it out.

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.

Enhance client side development experience
2 participants