Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new Development Tool page for parsing URLs and editing query parameters, wiring it into the existing tool routing + metadata system. The PR also introduces a Unix Timestamp Converter tool (routes + metadata) and updates dependencies/lockfile accordingly.
Changes:
- Added
UrlParserUI for URL component breakdown + query parameter editing. - Registered
/url-parserroute and added tool metadata/SEO content for the new page. - Introduced a Unix Timestamp Converter route/component and added
dayjs(plus substantial lockfile updates).
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Adds dayjs; dependency list change also results in lockfile churn. |
| package-lock.json | Large lockfile update including dayjs/curlconverter entries and many peer flag changes. |
| app/libs/developmentToolsConstant.tsx | Adds metadata/SEO content for url-parser and unix-timestamp-converter. |
| app/libs/constants.tsx | Registers new PATHS + routes for URL Parser and Unix Timestamp Converter; adds URL Parser to category listing. |
| app/components/developmentToolsComponent/urlParser.tsx | New URL Parser & Query Editor component implementation. |
| app/components/developmentToolsComponent/epochConverter.tsx | New Unix Timestamp Converter component using dayjs + Ant Design. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 'Debugging long URLs with multiple query parameters can be tedious and prone to errors. This tool simplifies the process by breaking down the URL into readable parts: Scheme, Host, Path, Port, and Hash.', | ||
| }, | ||
| { | ||
| description: | ||
| 'It features a dynamic table for editing query parameters, where you can add, modify, delete, and toggle encoding for individual parameters, seeing the URL update in real-time.', | ||
| }, |
There was a problem hiding this comment.
The tool description claims you can “toggle encoding for individual parameters”, but the URL Parser component doesn’t implement any encoded/decoded toggle (and QueryParam.active is never exposed in the UI). Either implement the encoding toggle control or adjust this copy so the UI and SEO text match actual behavior.
| import React, { useState, useEffect } from "react"; | ||
| import { Button, Input, DatePicker, message, Tooltip, Card, Typography, Space, Row, Col, ConfigProvider, theme } from "antd"; | ||
| import dayjs, { Dayjs } from "dayjs"; | ||
| import relativeTime from "dayjs/plugin/relativeTime"; | ||
| import utc from "dayjs/plugin/utc"; | ||
| import timezone from "dayjs/plugin/timezone"; | ||
| import DevelopmentToolsStyles from "../../developmentToolsStyles.module.scss"; | ||
| import { CopyOutlined, ReloadOutlined, DeleteOutlined } from "@ant-design/icons"; | ||
|
|
There was a problem hiding this comment.
This component has several unused imports/values (e.g., useEffect, Tooltip, Card, Title, message, CopyOutlined, and the copyToClipboard helper). With next lint enabled, unused imports are likely to produce lint errors/warnings and they also bloat the client bundle. Remove unused imports and dead code, or wire them into the UI if they’re intended.
| import XorCalculator from '../components/developmentToolsComponent/xorCalculator'; | ||
| import CurlToCodeConverter from '../components/developmentToolsComponent/curlToCodeConverter'; | ||
| import YAMLFormatterAndBeautifier from '../components/developmentToolsComponent/yamlFormatterAndBeautifier'; | ||
| import EpochConverter from '../components/developmentToolsComponent/epochConverter'; |
There was a problem hiding this comment.
PR description/issue scope is for the URL Parser tool, but this diff also introduces a new Unix Timestamp Converter route/component (EpochConverter, new PATH, and new route). If this is intentional, the PR description should be updated and the change ideally split into a separate PR to keep review + release scope clear.
| "antd": "5.16.2", | ||
| "curlconverter": "^4.12.0", | ||
| "bcryptjs": "^2.4.3", | ||
| "curlconverter": "^4.12.0", |
There was a problem hiding this comment.
curlconverter is listed as a production dependency and the lockfile pulls in tree-sitter/native install scripts, but there are no references to curlconverter anywhere in the repo code (no imports/requires). If it’s not actually used, consider removing it to reduce install size and avoid native build failures in CI/hosting environments.
| "curlconverter": "^4.12.0", |
| CRONTAB_GENERATOR: '/crontab-generator', | ||
| MORSE_CODE_TRANSLATOR: '/morse-code-translator', | ||
| UNIX_TIMESTAMP_CONVERTER: '/unix-timestamp-converter', | ||
| CURL_TO_CODE_CONVERTER: '/curl-to-code-converter', | ||
| }; |
There was a problem hiding this comment.
PATHS.UNIX_TIMESTAMP_CONVERTER and its route are registered, but the tool isn’t added anywhere in developmentToolsCategoryContent (which is used to build the homepage/search list). If this tool is meant to be discoverable like the others, add it to the category content; otherwise the route will exist but won’t appear in the tools list/API.
| const updateStateFromUrl = (url: URL) => { | ||
| setParsedUrl(url); | ||
| const params: QueryParam[] = []; | ||
| url.searchParams.forEach((value, key) => { | ||
| // We use random id for keys to allow duplicate keys in React list if necessary, | ||
| // though URLSearchParams handles them. | ||
| params.push({ | ||
| id: Math.random().toString(36).substr(2, 9), | ||
| key, | ||
| value, | ||
| active: true | ||
| }); |
There was a problem hiding this comment.
updateStateFromUrl assigns a new random id to every query param on each parse. Because urlInput is updated from the table editor (which triggers the parsing useEffect), this will regenerate row keys while the user is typing in the table and can cause input focus/caret to jump and edits to apply to the wrong row. Use stable IDs (e.g., preserve existing IDs by index, or keep queryParams as the source of truth and only parse on direct textarea edits).
| newUrl.search = newSearchParams.toString(); | ||
|
|
||
| // We update the input to reflect the changes. | ||
| // Note: This triggers the useEffect above, which re-parses. | ||
| // This circular dependency is usually handled by React batching or check for equality, | ||
| // but to be safe and avoid cursor jumping or loops, we might need a flag or careful management. | ||
| // However, for this simple tool, updating the main input is the "source of truth". | ||
|
|
||
| // To avoid re-rendering loop issues or cursor jumps if we were typing in the main input, | ||
| // we only update if the string is different. | ||
| if (newUrl.toString() !== urlInput) { | ||
| setUrlInput(newUrl.toString()); | ||
| } |
There was a problem hiding this comment.
updateUrlFromParams writes back into urlInput, which triggers the [urlInput] parsing effect and rebuilds queryParams. This creates an unnecessary re-parse loop and contributes to UI jitter (cursor jumps) when editing params/components. Consider splitting state so that textarea edits trigger parsing, while table/component edits update parsedUrl/queryParams directly without re-parsing (or gate the effect with a ref flag).
SyedFahad7
left a comment
SyedFahad7
left a comment
There was a problem hiding this comment.
Please sync your PR branch with the latest develop before re-review.
Use rebase (preferred) instead of merging develop into your feature branch to keep history clean:
git fetch upstream
git rebase upstream/develop
Resolve conflicts, then run local checks (npm run lint and npm run build)
git push --force-with-lease origin
Notes:
No need to fix unrelated pre-existing warnings across the repo.
Do fix any errors introduced by your PR (especially in touched files).
Rebase rewrites only your PR branch history; it does not rewrite our develop branch.
Once updated, we’ll re-review quickly.
udaykiran243
force-pushed
the
feat/url-parser-51
branch
from
3b88c7b to
74235fe
Compare
|
hey @SyedFahad7 made the changes, please review. |
SyedFahad7
left a comment
SyedFahad7
left a comment
There was a problem hiding this comment.
Great work on this feature, the UI and parsing flow are solid overall. A few things to address before we merge:
-
updateStateFromUrlregenerates a randomidon every parse this causes row key instability and input cursor/focus jumps while editing. -
updateUrlFromParamswrites tourlInput, which retriggers the[urlInput]effect , this causes unnecessary re-parsing/jitter. Please gate this flow or split textarea parsing from table edits. -
PATHS.UNIX_TIMESTAMP_CONVERTERis routed, but the tool is missing fromdevelopmentToolsCategoryContent, so it may not appear in homepage/search listings. -
Copy says users can "toggle encoding" but no encoding toggle exists in the UI (
QueryParam.activeis not exposed). Please either implement it or update the copy to match the actual behavior. -
Duplicate key in
constants.tsxtheCategory177appears twice in the object. In JavaScript/TypeScript, duplicate keys are invalid and one would silently overwrite the other at runtime. Please rename your entry toCategory178, :
Category177: [
{ url: '/existing-tool', title: 'Existing Tool', ... },
{ url: '/url-parser', title: 'URL Parser & Query String Editor', ... },
],Minor fix: Consider automatically URL-encoding values like spaces so the generated URL stays standards-compliant.
once these are fixed, this should be in good shape to merge!
|
hey @SyedFahad7 made the changes. |
|
Great work @udaykiran243, this is merge-ready from my side! ✅ One optional hardening suggestion (not blocking): In Consider switching to stable IDs instead, for example by preserving identity via index/key match or using a deterministic Again, not a blocker.. just a nice-to-have for long-term robustness! |
|
hey @SyedFahad7 made the request suggestion, requesting for a review. |
SyedFahad7
left a comment
SyedFahad7
left a comment
There was a problem hiding this comment.
Looks good to me now.
The stable ID follow up is implemented correctly in urlParser.tsx using param.${index}.${key} inside updateStateFromUrl. This addresses the earlier non blocking concern about random IDs causing unnecessary remount and re render behavior.
I also re ran local checks on this branch. Both npm run lint and npm run build pass successfully.
Approved for merge.
SyedFahad7
left a comment
SyedFahad7
left a comment
There was a problem hiding this comment.
Thanks for the update @udaykiran243 - The URL Parser + Query Editor work is in good shape.
What looks good
- URL Parser & Query Editor feature is implemented end-to-end and wired correctly.
- Prior URL parser issues appear addressed:
- constants duplication fixed
- encoding toggle behavior improved
- parsing flow is more stable
- Stable query param IDs are now deterministic in
updateStateFromUrl(param-${index}-${key}), which helps prevent unnecessary remounts/focus loss in the query table. - Unix Timestamp Converter is also correctly added and routed.
Requested changes (Unix converter)
-
epochConverter.tsxcurrently validates withNumber(val)(app/components/developmentToolsComponent/epochConverter.tsx:35), which accepts non-standard timestamp formats like" ","1e3", and"0x10".
Please enforce plain integer input for Unix timestamps (e.g.^-?\d+$) before conversion. -
On invalid input,
dateis not cleared (app/components/developmentToolsComponent/epochConverter.tsx:36-43path), so the DatePicker can show stale previous state while output saysInvalid Timestamp(app/components/developmentToolsComponent/epochConverter.tsx:197).
Please clear/resetdatewhen input is invalid to keep UI state consistent.
Non-blocking cleanup
- Remove unused imports/functions in
epochConverter.tsx(useEffect,Tooltip,Card,Title,CopyOutlined,copyToClipboard) to reduce noise.
Also:
Sync your branch with the latest develop using rebase instead of merge. we merged a pr a while go.
Example workflow:
git fetch upstream
git rebase upstream/develop
resolve conflicts if any
git push --force-with-lease origin
Overall: URL Parser is good to merge, but I recommend fixing the two Unix timestamp logic issues above before final approval.
Sorry for the inconvenience, the unix commit should be removed here, I’ll remove that commit. And this can be approved. |
|
No issues, you do not need to remove the Unix timestamp commit. I had mentioned that earlier because there was another PR for it, but I have already closed it. You can keep the Unix Timestamp Converter in this PR with the URL Parser and Query Editor and we will close both issues together. Just make sure the fixes mentioned in the review are addressed. |
sorry, but as they are different issues, if I close them in one PR I will loose points in Apertre, I was thinking about that 😅 |
|
It’s okay, I’ll close here, and I request you to add a hard label and remove the medium. |
No worries, we can close two issues in one PR. You will still get the points in Apetre for both issues. |
But how? |
Both features, Unix Timestamp Converter and URL Parser with Query Editor, are medium complexity. That is why the PR is labeled as medium. |
You can reference both issues in the PR description. For example: When the PR is merged, GitHub will automatically close both issues, so you still get credit for both. |
Yes, I agree with that, but if I close both of them in one PR I only get 7 points, instead of 14 points. |
No, in apertre only the PR counts not the closed issues |
|
It’s about how many merge PR, that I had made |
|
So, I’ll close both of them here and want you to label as hard. So this can be helpful getting 10points instead of 14. |
You can drop the unix timestamp commit from this pr. I've reopened the original unix convertor pr so you push changes for the issues i mentioned in that pr #16. Let this PR only contain the work for URl Parser & Query Editor. Both the tools fall under medium complexity. That is why the PR is labeled as medium. Hard is usually reserved for more complex implementations. |
okay sure 👍, thanks. |
|
Sorry if I made any trouble. |
udaykiran243
force-pushed
the
feat/url-parser-51
branch
from
7f76301 to
cfe71a4
Compare
|
hey @SyedFahad7 please review, I have removed the #28 commits |
@SyedFahad7 these are the changes I need to do in #28 right ? |
yea as you said you wanna maintain 2 PR's |
SyedFahad7
left a comment
SyedFahad7
left a comment
There was a problem hiding this comment.
The scope now looks clean and URL-parser-only.
I found one blocking issue to fix before approval:
app/libs/developmentToolsConstant.tsxstill contains a leftover merge-conflict marker:
>>>>>>> 74235fe (feat(tools): add visual URL Parser and Query Editor #51)
Please remove that marker and ensure the file ends with valid object syntax only.
Once that is cleaned, I can re-check and approve.
yes, made the changes in #28 |
@udaykiran243 fix this issue and push to the branch and we can merge then. |
meanwhile could you please review #28 |
Yes, LGTM. Have merged it. Rebase the latest develop to this branch as well before you push changes as I'm already seeing conflict issues with the latest develop. |
|
@udaykiran243
rebase your branch on latest |
on it, I was resolving the conflict and thanks for that. |
udaykiran243
force-pushed
the
feat/url-parser-51
branch
from
41c6c25 to
99f5839
Compare
|
hey @SyedFahad7 made the changes, requesting for a review. |
SyedFahad7
left a comment
SyedFahad7
left a comment
There was a problem hiding this comment.
Thanks for the updates, I rechecked & requested changes are now implemented correctly.
Approved from my side.
|
🎉 This PR is included in version 1.4.0-develop.8 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Feature: URL Parser & Query Editor
Resolves #51
Description
This PR introduces a new development tool, the URL Parser & Query String Editor, designed to simplify the process of debugging and constructing complex URLs.
It provides a visual interface to:
Implementation Details
app/components/developmentToolsComponent/urlParser.tsx- Implements the parsing logic using the nativeURLAPI and manages state for two-way binding between the raw URL string and the visual editor./url-parserroute inapp/libs/constants.tsx.app/libs/developmentToolsConstant.tsxwith tool descriptions, SEO metadata, and usage guides.✅ Checklist
Preview
Screen.Recording.2026-03-02.154941.mp4