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

Typed api.storeResult #8732

Merged
merged 2 commits into from
Jan 6, 2023
Merged

Typed api.storeResult #8732

merged 2 commits into from
Jan 6, 2023

Conversation

mischnic
Copy link
Member

@mischnic mischnic commented Dec 31, 2022

Not sure if this is actually a good idea, but I find it very confusing and error-prone that these two cases typecheck just fine

	// Does returns undefined if uncached, but returns a number if cached
    let requestB: void = await tracker.runRequest({
      id: 'abc',
      type: 'mock_request',
      run: async ({api}) => {
        api.storeResult(123);
      },
      input: null,
    });
	// Actually returns undefined if cached
    let requestB: string = await tracker.runRequest({
      id: 'abc',
      type: 'mock_request',
      run: async ({api}) => {
        return 'b'
      },
      input: null,
    });

@parcel-benchmark
Copy link

parcel-benchmark commented Dec 31, 2022

Benchmark Results

Kitchen Sink ✅

Timings

Description Time Difference
Cold 1.74s -57.00ms
Cached 384.00ms -1.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

Bundle Size Difference Time Difference
dist/legacy/parcel.7cdb0fad.webp 102.94kb +0.00b 106.00ms -172.00ms 🚀
dist/legacy/parcel.7cdb0fad.webp 102.94kb +0.00b 107.00ms -172.00ms 🚀
dist/modern/parcel.7cdb0fad.webp 102.94kb +0.00b 106.00ms -173.00ms 🚀
dist/legacy/index.b8ae99ba.css 94.00b +0.00b 301.00ms +18.00ms ⚠️
dist/modern/index.31cedca9.css 94.00b +0.00b 300.00ms +17.00ms ⚠️

React HackerNews ✅

Timings

Description Time Difference
Cold 11.26s -48.00ms
Cached 508.00ms -8.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

AtlasKit Editor ✅

Timings

Description Time Difference
Cold 1.94m -1.24s
Cached 2.57s +57.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

Three.js ✅

Timings

Description Time Difference
Cold 8.15s -77.00ms
Cached 307.00ms -8.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

Click here to view a detailed benchmark overview.

@mischnic
Copy link
Member Author

mischnic commented Jan 6, 2023

(One unsoundness does remain: runRequest() can still return undefined if the request didn't do .storeResult(), but enforcing that isn't possible with Flow, and the alternative of making the result always ?TResult is very unergonomic.)

@mischnic mischnic merged commit c97cf38 into v2 Jan 6, 2023
@mischnic mischnic deleted the typed-storeResult branch January 6, 2023 19:36
marcins pushed a commit to marcins/parcel that referenced this pull request Jul 14, 2023
* upstream/v2: (33 commits)
  v2.8.3
  Changelog for v2.8.3
  Address bug by updating an asset reference and merge conditions (parcel-bundler#8762)
  Fix CSS order when merging type change bundles (parcel-bundler#8766)
  fixing failing build for contributors on Linux using Node 18 (parcel-bundler#8763)
  Extension: Importers View and separate LSP protocol package (parcel-bundler#8747)
  Bump swc to fix sourcemaps with Windows line endings (parcel-bundler#8756)
  Apply HMR updates in topological order (parcel-bundler#8752)
  Make extension packaging work (parcel-bundler#8730)
  Typed api.storeResult (parcel-bundler#8732)
  Refactor LSP to use vscode-jsonrpc (parcel-bundler#8728)
  Bump swc (parcel-bundler#8742)
  Recursively check reachability when removing asset graphs from bundles in deduplication (parcel-bundler#6004)
  Fix tsc sourcemaps metadata (parcel-bundler#8734)
  Assigning to `this` in CommonJS (parcel-bundler#8737)
  Don't retarget dependencies if a symbol is imported multiple times with different local names (parcel-bundler#8738)
  Add a note about using flow in CONTRIBUTING.md (parcel-bundler#8731)
  filter out title execArgv to workers (parcel-bundler#8719)
  Document more of the BundleGraph class (parcel-bundler#8711)
  Fixed the hmr connection with host 0.0.0.0 (parcel-bundler#7357)
  ...
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.

3 participants