developertools-tech / developertools.tech

A collection of tools for developers served as a PWA
https://www.developertools.tech
MIT License
39 stars 45 forks source link

feat: add regex testing tool #52

Closed gaston-flores closed 1 year ago

gaston-flores commented 1 year ago

Type of Pull Request

Related Issue #s or links (if any): #9

Description of Changes

This PR adds a Regex testing tool:

image
netlify[bot] commented 1 year ago

Deploy Preview for developertools-tech ready!

Name Link
Latest commit 6f7554580adf23c7002fafec07f76aa191144d5b
Latest deploy log https://app.netlify.com/sites/developertools-tech/deploys/6344675c3aa9fc000823e6d7
Deploy Preview https://deploy-preview-52--developertools-tech.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

gaston-flores commented 1 year ago

Also just FYI @dlford I think you are missing a key here, I'm getting this warning on my console:

Warning: Each child in a list should have a unique "key" prop. See https://reactjs.org/link/warning-keys for more information.
    at a
    at Layout (webpack-internal:///./components/Layout.tsx:124:19)
    at IndexPage
    at InnerThemeProvider (/workspaces/developertools.tech/node_modules/@mui/system/ThemeProvider/ThemeProvider.js:29:39)
    at ThemeProvider (/workspaces/developertools.tech/node_modules/@mui/private-theming/node/ThemeProvider/ThemeProvider.js:55:5)
    at ThemeProvider (/workspaces/developertools.tech/node_modules/@mui/system/ThemeProvider/ThemeProvider.js:49:5)
    at MyApp (webpack-internal:///./pages/_app.tsx:39:13)
    at StyleRegistry (/workspaces/developertools.tech/node_modules/next/dist/styled-jsx/dist/index/index.js:671:34)
    at AppContainer (/workspaces/developertools.tech/node_modules/next/dist/server/render.js:280:29)
    at AppContainerWithIsomorphicFiberStructure (/workspaces/developertools.tech/node_modules/next/dist/server/render.js:309:57)
    at div
    at Body (/workspaces/developertools.tech/node_modules/next/dist/server/render.js:596:21)
dlford commented 1 year ago

Also just FYI @dlford I think you are missing a key here, I'm getting this warning on my console:

Warning: Each child in a list should have a unique "key" prop. See https://reactjs.org/link/warning-keys for more information.
    at a
    at Layout (webpack-internal:///./components/Layout.tsx:124:19)
    at IndexPage
    at InnerThemeProvider (/workspaces/developertools.tech/node_modules/@mui/system/ThemeProvider/ThemeProvider.js:29:39)
    at ThemeProvider (/workspaces/developertools.tech/node_modules/@mui/private-theming/node/ThemeProvider/ThemeProvider.js:55:5)
    at ThemeProvider (/workspaces/developertools.tech/node_modules/@mui/system/ThemeProvider/ThemeProvider.js:49:5)
    at MyApp (webpack-internal:///./pages/_app.tsx:39:13)
    at StyleRegistry (/workspaces/developertools.tech/node_modules/next/dist/styled-jsx/dist/index/index.js:671:34)
    at AppContainer (/workspaces/developertools.tech/node_modules/next/dist/server/render.js:280:29)
    at AppContainerWithIsomorphicFiberStructure (/workspaces/developertools.tech/node_modules/next/dist/server/render.js:309:57)
    at div
    at Body (/workspaces/developertools.tech/node_modules/next/dist/server/render.js:596:21)

Yes I saw that too, not sure how it got in there but I'll track it down, thanks for calling it out!

gaston-flores commented 1 year ago

Ooooh, gotcha! If you don't mind I think those would be best for a separate PR. I could take a crack at adding the flags, but my solution for showing the match/capture groups would need some refactoring to work with the g flag.

dlford commented 1 year ago

@gaston-flores I don't mind, you've already exceeded the requirements in the original issue.

dlford commented 1 year ago

@gaston-flores it's better to hash the content of a node than use the array index, an example can be found here - https://github.com/developertools-tech/developertools.tech/blob/main/pages/lorem-ipsum.tsx#L231

dlford commented 1 year ago

Also, thanks for adding the global flag, looks great!

gaston-flores commented 1 year ago

Oh, gotcha! I'll give it a try! I don't know why, but the eslint errors are not showing up in my local console. Thanks!

dlford commented 1 year ago

I recently changed the eslint config because it was missing a line for the airbnb rules, maybe your local copy doesn't have that change yet?

The hashing bit helps keep each node unique in an array, in some cases using the array index can cause weird behavior (e.g. replacing the content of a node may not render correctly because the key didn't change).

gaston-flores commented 1 year ago

Yep, works now that I rebased the branch!

I'm kinda running into strange issues when using hash though:

https://user-images.githubusercontent.com/18354222/194915400-3aeb9730-89bf-40e8-bc34-7577128b4c6a.mov

It seems tying the array key to testCase creates a new input on every keypress, losing focus in the process. Here's what I'm doing:

{testCases.map((testCase, index) => {
  const id = hash(testCase);
  return (
    <RegexTestCase
      key={id}
      index={index}
      testCase={testCase}
      regex={regex}
      onDelete={handleRemoveTestCase}
      onInput={handleTestCaseInput}
      deleteDisabled={testCases.length === 1}
    />
  );
})}

I'll leave it with disable-next-line for now.

dlford commented 1 year ago

@gaston-flores Oh yeah, that's not good. Go ahead and disable the eslint rule at the top of the file then and use the array index, please add a note on the next line // TODO: Find a better way to replace array index keys, or something of that nature. I know in the past I've created an ID dynamically and assigned it to each node, not sure how I did it off the top of my head.

In any case, if the array index is working, I'm fine with it for now.

Thanks!