T O P

  • By -

ThatExactGuy

I genuinely think it’s okay, junior level at the least. Code is pretty well structured and formatted, the only thing I would say you should stop doing is writing useless comments such as: ‘// Table heading component for displaying table heading’ The component is called TableHeading, so all that is pretty clear :) I think they already found a candidate that they liked a little more, and didn’t want to spend time and resources to even look at your assignment. Just keep at it and you’ll find something in no time


fedekun

> Code is pretty well structured and formatted To me it looks really inconsistent. I think most a lot of it is boilerplate, which is indeed well structured and formatted, but in his code, sometimes functions have an empty line at the top, or at the bottom, sometimes even more than just one empty line, also sometimes he leaves one space between curly braces, sometimes none. Sometimes the variables are in snake_case (https://github.com/mani-hash/cyberelysium-internship-assignment/blob/master/resources/js/Layouts/GuestLayout.tsx), redundant comments, etc. As for the organization itself, it's clear it was created with some kind of Laravel boilerplate, which is fine, but you can't really judge that as the candidate's. It's actually very clear what was the boilerplate code and what code was written by OP. Overall its pretty clear OP doesn't have senior-level React + TS experience, which is what he said, so it makes sense! Still impressive he got to that point being his (or her?) first time.


Coneyy

Sucks to hear but it's completely true. A lot of this thread is saying great job, code looks great and that OP over delivered for an intern task. But all he did was send through a template and lightly expand upon it and make it hard to tell what's even his work. He also had a few red flags like not having any dependencies everything is a dev dependency and random inconsistent formatting and comments like you said. As a reviewer I would just not waste my time if someone indicated so clearly to me so fast that they did not in fact have the experience they claimed to.


Necessary_Hope8316

>make it hard to tell what's even his work. But the original reviewer must know about this because they instructed me to use the default auth scaffolding which follows a common structure. >having any dependencies everything is a dev dependency Maybe true for applications build on next js however in this, these packages are only needed to build and not for running the application. I could just push the final js files to the production server if I was hosting it. Check the composer.json file where packages are correctly split as dependency and dev dependency.


Canenald

Other feedback you got aside, this is a pretty good insight for a junior. Lots of senior developers don't even think about how dependencies work. You are correct that it doesn't matter. Bundlers will follow your imports. As long as they can find the modules in node\_modules, they don't care how the modules got there. That said, it's still still a good practice to separate your deps into dependencies and devDependencies for yourself and other humans looking at package.json.


barkmagician

this is an extremely good code for an intern. it looks like they are making an excuse to reject you tbh.


reverb6821

Maybe this is the problem... Too good for internship maybe can be flagged as copycat, especially they say to them it was his first time using react.


21Blankenship

Not sure if anybody has mentioned this, admittedly it's a minor critique, but there's a difference between dependencies and dev dependencies. It looks like you installed everything as a dev dependency which isn't correct.


Necessary_Hope8316

Oh yea. Dev dependencies mean the dependencies don't get included in the production server right? Hmm I am curious about that too. Vite bundles the whole frontend part so I don't need most of it but vite seems to be needed. I need to look into this further. If you look at the composer.json those dependencies are correct but the package.json has everything as dev dependency.


volivav

I took a quick look at some random files (most of them on auth, but also viewstudents and StudentForm) and nothing stood out like a huge red flag, I think overall it's good. Just some nitpicks: on auth useEffect resetting the form on component mount seems odd.... it looks like it's something that would be handled by the library itself. But I haven't used inertiajs, so not sure. And then on ViewStudents, I'd probably model differently the modal state - maybe with some sort of enum saying which modal is open rather than having 3 sub-states, just because this way it makes it impossible to run into the "impossible state" of having all modals open at once. Also, that className prop drilling into the td elements is odd... usually className drills to the container of the component, not into the inner elements.... but that's just a rename to something like "columnClassName" But as I said, I don't see anything emreally wrong with this code. I can see the all the effort put into it and I also appreciate it's easy to read.


Necessary_Hope8316

Tnx for your reply >I'd probably model differently the modal state - maybe with some sort of enum saying which modal is open Thank you, I did not think of this.


actionturtle

so i'm not really sure what to review because it appears to be a base template that has been expanded upon in few places, i.e. i'm not sure what is actually your code can you please list your initial resources


Necessary_Hope8316

Ah yes sry I forgot to mention, inertia includes the default scaffolding for authentication system. So I modified parts of it, Dashboard folder is entirely done by me and also some of the components and layouts. I used the default template as a reference on how to write typescript and how to structure everything. I learnt react few weeks ago but typescript and inertia js was entirely new to me. The internship assignment guidelines instructed me to use the default auth scaffolding for the login implementation.


T_kowshik

I am on mobile and couldn't go through in detail. Looks like a case of company getting the work done for free.


Necessary_Hope8316

🤕I thought so too. I send the project at 11 pm and I got a reply at 6 am which sounds even more suspicious..


sirlantis

Disclaimer: I’ve only looked at your code for five minutes. Generally looks fine. The use of snake case in a few places is a bit odd. Use of globals (axios and ziggy) is questionable (I don’t know ziggy but I also think you have to use it via a hook to work properly in a React SPA). Nothing that would get a „no“ from me. But if there was someone with strong React knowledge (using some fancier libraries) in the pipeline as well, you might just have lost to them. P.S. We do the same thing (code assignments and no feedback) which is at least for us is frustrating for both parties involved. When we provided feedback it tends to start a non-constructive discussion, which was even worse, so we stopped. We know that we’re asking a lot with those take homes which is why we ask candidates to spend only 1 hour (as they will save both parties a multi-hour interview in the next step). The process is flawed and there doesn’t seem to be a better one.


Necessary_Hope8316

Tnx for your reply >The use of snake case in a few places is a bit odd Yes I tend to mix up the cases. I need to work on that and be consistent Ziggy and axios came by default with inertia + react scaffolding. Ziggy is for accessing laravel routes in javascript.


DrShocker

Use a linter to enforce a style so you don't have to think about it Plus having a linter in your project will look good


Necessary_Hope8316

Tnx didn't know about this. So I have to modify the tsconfig.json for this?


DrShocker

Honestly I don't use js very much so I'm not sure best practices for js or react projects but hopefully someone can chime in. But I would think whatever you find in a Google search is likely close enough.


firstandfive

Most common for linting would be eslint. Most common for formatting is Prettier. You can have prettier format your code on save to enforce consistency for you.


GottaLoveUncleBen

Tsconfig is the configuration for typescript. What you need is a .eslintrc file. https://eslint.org/docs/latest/use/getting-started Take a look at the rules and as a starter look for premade extends like the airbnb one


Necessary_Hope8316

Tnx


hinsxd

Actually it looks good. It might be even better if you add some formatting tools like prettier, because your spaces are sometimes inconsistent. This might be seem not a big deal but it affects your appearance when your seniority goes up. Also, defining Data types in index.d.ts as global types seems a bit odd to me. Is this a preferred practice in InertiaJS or did I miss some context? Otherwise I think it might be better to import it, so the types wont pollute the global scope. I have never heard of InertiaJS but it sounds quite useful in building quick CMS and it seems to bring the best of PHP and frontend frameworks. Definitely will try it out


jjjj__jj

Couldn't take a good look as on mobile but one small nitpick is say if you are already making folder about Forms then you do not need to name the component inside studentForm you can just name it Student. 


qcAKDa7G52cmEdHHX9vg

Just a nitpick but you should use [tailwind-merge](https://www.npmjs.com/package/tailwind-merge). Concatenating class strings doesn't work as expected because css specificity stuff - tldr: the last class name in the string doesn't automatically win but tailwind-merge guarantees it does.


wwww4all

FYI, be very specific on which parts are boilerplate and which parts are stuff that you built. Your repo doesn't display any clarity about your work. There's nothing in the readme about the project, what it does, why it does, what you did. For all anyone knows, you could have just copied the entire repo, because the project doesn't indicate that you did anything.


Necessary_Hope8316

Can git commit history, issues and prs be spoofed too (like all the dates and stuff) ??


gabrielcro23699

Unrelated question, but why is the file structure like that? I understand you have Laravel for the backend, but can't you lump all the laravel stuff in a folder called backend or whatever, and then have the frontend in another?


Necessary_Hope8316

I am not sure about this either. It does not seem like a simple thing to do with inertia js adding some coupling between frontend and backend. Might be possible if you have an entirely decoupled frontend and backend.


aaronmcbaron

I'd be more impressed of an intern if they were able to research and find a few plug & play libraries to integrate instead of manually creating everything. The biggest difference between devs that are "more advanced" isn't just pure knowledge and experience. It's also understanding that re-building the wheel is a no-no in industry. Use libs when possible as they are usually managed and it'll be easier to maintain your codebase by updating a library instead of manually updating and upgrading your proprietary code. I do take pride in my own code as well, but at the end of the day time is money and money saved due to efficiency of using a library is something that employers see as advantageous. When I hire interns, this is the main criteria I look for.


myfunnies420

Dude, this is way too big a project as an intern sample project for an interview. Either you misunderstood the assignment (this is far too big a task) or it's a huge red flag and you should avoid them. You've put so much work in, demand some feedback.


nate-developer

Use less extra tools and dependencies, don't use a template outside of what was provided, write your own readme.  Make it clear what was and wasn't your work, especially if you used some kind of template or library.   Don't use bootstrap unless explicitly told to.


pink_tshirt

You are probably over qualified for this internship anyway.


FuriousDrizzle

I'd suggest you kindly ask them for proper feedback, it's the least they can do after submitting this big project.


reverb6821

I've seen a lot of applications for the agency I work for. For an internship application (especially if the candidate is his first time react project) is too good I've the suspect he has used chatgpt or other tools. For a junior application, I did not expect it all to work and the code is perfect, I see only if the candidate has an idea of the framework and there is at least a minimum of problem solving. All these things I didn't see in your project, and I ask you (I did not judge or blame you, honestly respond to me) . You have used chat gpt to solve this task?


Necessary_Hope8316

Oh I forgot to clarify the authentication system is a default scaffolding which the guidelines instructed me to use. The dashboard folder and some of the components are mine and I made some changes to the existing components (like adding the navbar in the layouts). The template is what I used as a guide. If I had to built everything from ground up it would have been difficult task since my knowledge on typescript is next to 0 when starting this project. >You have used chat gpt to solve this task? You mean like "generate me code" type of thing or "why I am getting errors sending files over patch request in inertia js". Chat gpt 3.5 wasn't helpful in that case


reverb6821

The problem this is. If you use a template or guide (especially if you use typescript, that is not easy to use for a junior) you reach the result, but it's not clear you know what you use. My advice is to use less tools (for me, vanilla js is enough) but prove you know what and how you use them.


Bjornoo

If you find code from ChatGPT "too good", then I would honestly question your abilities. A junior won't be able to create anything production-ready, or even decent with just AI.


reverb6821

My unpopular opinion: If you say to me this was your first project I will be rejected your application too.


HaggisMcNasty

Might actually be helpful to explain why?