Skip to content

Conversation

@david-crespo
Copy link
Collaborator

@david-crespo david-crespo commented Mar 18, 2024

Closes #2070

  • Show v4 and v6 counts in a column on IP pools table
  • Fancy display logic for big numbers
  • Show v4 and v6 counts in properties table on IP pool detail
  • Make mock handler use the actual ranges
  • Put utilization on IP pool detail page
2024-03-20-ip-pool-utilization.mp4

@vercel
Copy link

vercel bot commented Mar 18, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
console ✅ Ready (Inspect) Visit Preview Mar 21, 2024 5:12pm

[23094304823948203952304920342n, '23.1E27'],
])('displayBigNum %d -> %s', (input, output) => {
expect(displayBigNum(input)).toEqual(output)
})
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

these bigint literals are the reason for the target: es2020 bump in the tsconfig

"sourceMap": true,
"strict": true,
"target": "es2015",
"target": "es2020",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is so we can use bigint literals, e.g., 1123901234n. target seems primarily relevant in that it determines lib if you're not also setting lib (we are not). There is no change in our bundle size from this, which is weird. Will add more detail to this comment later.

https://www.typescriptlang.org/tsconfig#target
https://www.typescriptlang.org/tsconfig#lib

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

According to the compat table (ironically, works poorly in FF but well in Chrome), these are the features for each version:

image

These are the ones for es2020 specifically, and they all have been supported since 2019 or early 2020.

I looked through most of the other features and I think we could go as high as es2022, but I don't think there's much advantage to doing that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah. Of course. This explains why it had no effect on bundle size.

We should also note that TypeScript's target only impacts your runtime code if you're using TypeScript to transpile your source files to JavaScript. If you're using another tool to transpile, such as Babel, ESBuild, or SWC, then that tool's settings will be what determines the syntax used in your output JavaScript code.

https://www.learningtypescript.com/articles/why-increase-your-tsconfig-target#target-might-not-impact-your-runtime-code

I'm embarrassed not to have known this, but here's how Vite handles it.

build.target

Browser compatibility target for the final bundle. The default value is a Vite special value, 'modules', which targets browsers with native ES Modules, native ESM dynamic import, and import.meta support. Vite will replace 'modules' to ['es2020', 'edge88', 'firefox78', 'chrome87', 'safari14']

Note the build will fail if the code contains features that cannot be safely transpiled by esbuild. See esbuild docs for more details.

https://vitejs.dev/config/build-options.html#build-target

Note that by default, Vite only handles syntax transforms and does not cover polyfills.

https://vitejs.dev/guide/build.html#browser-compatibility

The linked page from the esbuild docs about what features are in what version is better than anything I found yesterday while researching this stuff.

https://esbuild.github.io/content-types/#javascript

It also gave me a good example of too-new syntax to test it with:

x Build failed in 7.97s
error during build:
Error: [vite:esbuild-transpile] Transform failed with 1 error:
assets/app-!~{001}~.js:62912:0: ERROR: Top-level await is not available in the configured target environment ("chrome87", "edge88", "es2020", "firefox78", "safari14" + 2 overrides)

Top-level await is not available in the configured target environment ("chrome87", "edge88", "es2020", "firefox78", "safari14" + 2 overrides)
62910|  };
62911|  const colHelper = createColumnHelper();
62912|  await new Promise((res) => res(1));
   |  ^
62913|  function SiloAccessPage() {
62914|    const [addModalOpen, setAddModalOpen] = reactExports.useState(false);

Conclusion

So in theory, Vite has us covered for making sure we are okay on syntax, and TS target should prevent us from using new built-in things we can't use. That said, I couldn't find a good example of something like that being rejected by TS.

{allocated.toLocaleString()}
<div className="text-quaternary">{capacityLabel}</div>
<div className="!normal-case text-secondary">
{displayBigNum(capacity)}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This component is getting out of hand. I will make an issue about breaking it up into composable components.

const percentage =
typeof provisioned === 'bigint'
? (provisioned * 100n) / (capacity as bigint) // TS is being a jerk
: (provisioned * 100) / capacity
Copy link
Collaborator Author

@david-crespo david-crespo Mar 21, 2024

Choose a reason for hiding this comment

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

I’m going to pull this logic out into a function so I can unit test the hell out of it.

import { percentage, splitDecimal } from '~/util/math'

export const CapacityBar = ({
export const CapacityBar = <T extends number | bigint>({
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

generic is so we can let provisioned and allocated be number | bigint while enforcing that they are the same type

<IpUtilFrac {...ipv4} />
</div>
)
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This should be what users see because they almost certainly have no IPv6 ranges.

</div>
)
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

took a slightly different approach here than the possibly too clever "if there is IPv6, show both, otherwise just show IPv4 with no v4 label" in the table cell. Here we just show whichever ones are present, including possibly both or none.

Comment on lines +176 to +178
// the point of these tests is to make sure the toLowerCase shenanigan in
// toEngNotation doesn't go horribly wrong due some obscure locale's concept of
// engineering notation
Copy link
Contributor

Choose a reason for hiding this comment

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

Love that you included these

Copy link
Contributor

@charliepark charliepark left a comment

Choose a reason for hiding this comment

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

Really nice!

@david-crespo david-crespo enabled auto-merge (squash) March 21, 2024 17:19
@david-crespo david-crespo merged commit b844a42 into main Mar 21, 2024
@david-crespo david-crespo deleted the ip-pools-util branch March 21, 2024 17:20
david-crespo added a commit to oxidecomputer/omicron that referenced this pull request Mar 21, 2024
oxidecomputer/console@784e8aa...7c96844

* [7c968443](oxidecomputer/console@7c968443) oxidecomputer/console#2083
* [b844a42c](oxidecomputer/console@b844a42c) oxidecomputer/console#2078
* [47835ee3](oxidecomputer/console@47835ee3) oxidecomputer/console#2086
* [c4ddebc2](oxidecomputer/console@c4ddebc2) oxidecomputer/console#2082
* [530a0997](oxidecomputer/console@530a0997) upload-artifact@v4 requires unique filenames for each run in matrix
* [b996c34c](oxidecomputer/console@b996c34c) bump actions/upload-artifact to v4
* [bc51f530](oxidecomputer/console@bc51f530) bump API to latest main
* [743da3b0](oxidecomputer/console@743da3b0) oxidecomputer/console#2080
* [7596d633](oxidecomputer/console@7596d633) oxidecomputer/console#2073
* [65531bff](oxidecomputer/console@65531bff) oxidecomputer/console#2074
* [4d226cd3](oxidecomputer/console@4d226cd3) oxidecomputer/console#1891
* [7490104a](oxidecomputer/console@7490104a) update readme with cool new preview URL
* [a2218994](oxidecomputer/console@a2218994) oxidecomputer/console#2066
* [641ebe85](oxidecomputer/console@641ebe85) oxidecomputer/console#2061
* [1bb8dd3a](oxidecomputer/console@1bb8dd3a) oxidecomputer/console#2055
* [41dd9f06](oxidecomputer/console@41dd9f06) oxidecomputer/console#2059
* [04692802](oxidecomputer/console@04692802) oxidecomputer/console#2048
* [8c30305e](oxidecomputer/console@8c30305e) oxidecomputer/console#2050
* [53709d2a](oxidecomputer/console@53709d2a) oxidecomputer/console#2046
* [84caede7](oxidecomputer/console@84caede7) oxidecomputer/console#2026
* [bca9f42a](oxidecomputer/console@bca9f42a) oxidecomputer/console#2045
* [09e5f713](oxidecomputer/console@09e5f713) oxidecomputer/console#2043
* [d780e5d8](oxidecomputer/console@d780e5d8) fix casing on floating IP attach/detach toasts
david-crespo added a commit to oxidecomputer/omicron that referenced this pull request Mar 21, 2024
oxidecomputer/console@784e8aa...7c96844

* [7c968443](oxidecomputer/console@7c968443)
oxidecomputer/console#2083
* [b844a42c](oxidecomputer/console@b844a42c)
oxidecomputer/console#2078
* [47835ee3](oxidecomputer/console@47835ee3)
oxidecomputer/console#2086
* [c4ddebc2](oxidecomputer/console@c4ddebc2)
oxidecomputer/console#2082
* [530a0997](oxidecomputer/console@530a0997)
upload-artifact@v4 requires unique filenames for each run in matrix
* [b996c34c](oxidecomputer/console@b996c34c)
bump actions/upload-artifact to v4
* [bc51f530](oxidecomputer/console@bc51f530)
bump API to latest main
* [743da3b0](oxidecomputer/console@743da3b0)
oxidecomputer/console#2080
* [7596d633](oxidecomputer/console@7596d633)
oxidecomputer/console#2073
* [65531bff](oxidecomputer/console@65531bff)
oxidecomputer/console#2074
* [4d226cd3](oxidecomputer/console@4d226cd3)
oxidecomputer/console#1891
* [7490104a](oxidecomputer/console@7490104a)
update readme with cool new preview URL
* [a2218994](oxidecomputer/console@a2218994)
oxidecomputer/console#2066
* [641ebe85](oxidecomputer/console@641ebe85)
oxidecomputer/console#2061
* [1bb8dd3a](oxidecomputer/console@1bb8dd3a)
oxidecomputer/console#2055
* [41dd9f06](oxidecomputer/console@41dd9f06)
oxidecomputer/console#2059
* [04692802](oxidecomputer/console@04692802)
oxidecomputer/console#2048
* [8c30305e](oxidecomputer/console@8c30305e)
oxidecomputer/console#2050
* [53709d2a](oxidecomputer/console@53709d2a)
oxidecomputer/console#2046
* [84caede7](oxidecomputer/console@84caede7)
oxidecomputer/console#2026
* [bca9f42a](oxidecomputer/console@bca9f42a)
oxidecomputer/console#2045
* [09e5f713](oxidecomputer/console@09e5f713)
oxidecomputer/console#2043
* [d780e5d8](oxidecomputer/console@d780e5d8)
fix casing on floating IP attach/detach toasts
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.

Show IP pool utilization on IP pool list and detail

3 participants