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

flow typings for node's built-in Module module #5364

Closed
wants to merge 20 commits into from

Conversation

gorakong
Copy link
Contributor

@gorakong gorakong commented Nov 18, 2020

Test Plan:

  • verified that there were no flow errors

@height
Copy link

height bot commented Nov 18, 2020

Link Height tasks by mentioning a task ID in the pull request title or description, commit messages, or comments.

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

@gorakong gorakong changed the title flow typings for node's "Module" module flow typings for node's built-in Module module Nov 18, 2020
@mischnic
Copy link
Member

Maybe we should also try upstreaming this: https://github.com/facebook/flow/blob/05cd1bae10d89c9d4155ab99d2d1a9ed5eca9f9b/lib/node.js#L125-L130

@gorakong
Copy link
Contributor Author

@mischnic sounds good! will be following up with a submission to the Node.js libdef in the main flow repo

@parcel-benchmark
Copy link

parcel-benchmark commented Nov 19, 2020

Benchmark Results

Kitchen Sink ✅

Timings

Description Time Difference
Cold 6.48s -225.00ms
Cached 1.54s +6.00ms

Cold Bundles

Bundle Size Difference Time Difference
dist/legacy/parcel.d5807e82.webp 102.94kb +0.00b 140.00ms -37.00ms 🚀
dist/modern/parcel.d5807e82.webp 102.94kb +0.00b 279.00ms +85.00ms ⚠️
dist/legacy/index.79101d60.js 1.10kb +0.00b 2.01s -399.00ms 🚀
dist/modern/index.c851b754.js 1.10kb +0.00b 2.14s -303.00ms 🚀
dist/legacy/index.html 701.00b +0.00b 2.11s -301.00ms 🚀
dist/modern/index.html 701.00b +0.00b 2.23s -225.00ms 🚀
dist/legacy/index.b6f8331e.css 77.00b +0.00b 1.86s -506.00ms 🚀
dist/modern/index.a76d6f77.css 77.00b +0.00b 2.03s -319.00ms 🚀

Cached Bundles

Bundle Size Difference Time Difference
dist/legacy/parcel.d5807e82.webp 102.94kb +0.00b 1.10s -114.00ms 🚀
dist/legacy/index.79101d60.js 1.10kb +0.00b 1.21s -126.00ms 🚀
dist/legacy/index.html 701.00b +0.00b 1.17s -118.00ms 🚀
dist/modern/index.html 701.00b +0.00b 1.18s -116.00ms 🚀
dist/legacy/index.b6f8331e.css 77.00b +0.00b 1.17s -118.00ms 🚀
dist/modern/index.a76d6f77.css 77.00b +0.00b 1.17s -107.00ms 🚀

React HackerNews ✅

Timings

Description Time Difference
Cold 24.49s +1.03s
Cached 1.56s +42.00ms

Cold Bundles

Bundle Size Difference Time Difference
dist/NotFound.efcc5e9b.js 532.00b +0.00b 1.09s +209.00ms ⚠️
dist/logo.24c8bf9e.png 274.00b +0.00b 163.00ms +75.00ms ⚠️

Cached Bundles

Bundle Size Difference Time Difference
dist/index.js 478.31kb +0.00b 1.03s +73.00ms ⚠️
dist/PermalinkedComment.2e4627d5.js 4.22kb +0.00b 1.03s +72.00ms ⚠️
dist/UserProfile.0c18eb4e.js 1.70kb +0.00b 1.03s +72.00ms ⚠️
dist/NotFound.efcc5e9b.js 532.00b +0.00b 1.03s +72.00ms ⚠️
dist/logo.24c8bf9e.png 274.00b +0.00b 967.00ms +78.00ms ⚠️

AtlasKit Editor 🚨

Timings

Description Time Difference
Cold FAILED -0.00ms
Cached 4.35s -109.00ms

Cold Bundles

No bundles found, this is probably a failed build...

Cached Bundles

Bundle Size Difference Time Difference
dist/index.cc40525d.js 2.30mb +0.00b 466.00ms -25.00ms 🚀
dist/EmojiPickerComponent.8d912556.js 139.08kb +0.00b 703.00ms +41.00ms ⚠️
dist/esm.74ff7b72.js 27.74kb +0.00b 703.00ms +41.00ms ⚠️
dist/component.22b0fde0.js 22.51kb +0.00b 477.00ms -38.00ms 🚀
dist/js.0c4ce585.js 16.55kb +0.00b 477.00ms -38.00ms 🚀
dist/workerHasher.a22a1398.js 11.90kb +0.00b 703.00ms +41.00ms ⚠️
dist/component.601a47de.js 6.27kb +0.00b 480.00ms -40.00ms 🚀
dist/png-chunks-extract.08e8af36.js 3.55kb +0.00b 477.00ms -38.00ms 🚀
dist/Modal.fca4b016.js 3.15kb +0.00b 468.00ms -41.00ms 🚀
dist/16.29c076d0.js 2.49kb +0.00b 466.00ms -25.00ms 🚀
dist/images.f4282212.js 1.90kb +0.00b 620.00ms +41.00ms ⚠️
dist/feedback.946661fe.js 1.86kb +0.00b 683.00ms +36.00ms ⚠️
dist/16.07e17d98.js 1.79kb +0.00b 499.00ms -36.00ms 🚀
dist/workerHasher.0325314f.js 1.75kb +0.00b 703.00ms +41.00ms ⚠️
dist/list-number.605d56ab.js 1.68kb +0.00b 630.00ms +36.00ms ⚠️
dist/link.72eaa0b1.js 1.53kb +0.00b 620.00ms +36.00ms ⚠️
dist/heading6.7a56e9b9.js 1.53kb +0.00b 683.00ms +36.00ms ⚠️
dist/heading3.44db9957.js 1.51kb +0.00b 674.00ms +38.00ms ⚠️
dist/16.24210346.js 1.46kb +0.00b 499.00ms -36.00ms 🚀
dist/16.c087bab3.js 1.46kb +0.00b 502.00ms -41.00ms 🚀
dist/emoji.7fac1ca5.js 1.45kb +0.00b 620.00ms +41.00ms ⚠️
dist/heading5.7de42d71.js 1.40kb +0.00b 683.00ms +36.00ms ⚠️
dist/expand.b1f08bc4.js 1.38kb +0.00b 683.00ms +36.00ms ⚠️
dist/heading2.b89a5450.js 1.33kb +0.00b 674.00ms +38.00ms ⚠️
dist/mention.51f20c83.js 1.29kb +0.00b 630.00ms +36.00ms ⚠️
dist/Modal.df7b74de.js 1.28kb +0.00b 477.00ms -38.00ms 🚀
dist/layout.85f4f10e.js 1.27kb +0.00b 620.00ms +41.00ms ⚠️
dist/16.82077efb.js 1.26kb +0.00b 499.00ms -36.00ms 🚀
dist/16.1a3f7446.js 1.26kb +0.00b 499.00ms -36.00ms 🚀
dist/divider.a3b503cc.js 1.25kb +0.00b 620.00ms +41.00ms ⚠️
dist/component.9bdd6688.js 1.23kb +0.00b 466.00ms -25.00ms 🚀
dist/16.7ed14d80.js 1.23kb +0.00b 466.00ms -25.00ms 🚀
dist/list.2f77057e.js 1.18kb +0.00b 630.00ms +36.00ms ⚠️
dist/heading1.edaa8b50.js 1.18kb +0.00b 674.00ms +38.00ms ⚠️
dist/panel-error.a4fdbb96.js 1.11kb +0.00b 630.00ms +36.00ms ⚠️
dist/table.70f6910c.js 1.09kb +0.00b 674.00ms +38.00ms ⚠️
dist/simpleHasher.7d3e36e5.js 755.00b +0.00b 703.00ms +41.00ms ⚠️
dist/index.html 119.00b +0.00b 363.00ms +20.00ms ⚠️

Three.js x4 🚨

Timings

Description Time Difference
Cold FAILED -0.00ms
Cached FAILED -0.00ms

Cold Bundles

No bundles found, this is probably a failed build...

Cached Bundles

No bundles found, this is probably a failed build...

Click here to view a detailed benchmark overview.

@gorakong gorakong force-pushed the gkong/node-module-flow branch 3 times, most recently from 184b0b0 to 2b1e3d6 Compare November 19, 2020 01:09
declare type FilePath = string;

// Derived from https://github.com/facebook/flow/blob/61913b9f0dcd564fd4c74fefbe896b33ef447cab/lib/node.js
declare class Url {
Copy link
Contributor

Choose a reason for hiding this comment

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

import type {URL} from 'url'?

Copy link
Contributor

Choose a reason for hiding this comment

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

looks like this is addressed in the upstream PR :) facebook/flow#8540

@mischnic mischnic closed this Dec 21, 2022
@mischnic mischnic deleted the gkong/node-module-flow branch December 21, 2022 21:29
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.

4 participants