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

fix: Docs-site Shape Property page errors and crashes #1045

Merged
merged 6 commits into from
Jun 7, 2022
Merged

Conversation

cmumatt
Copy link
Contributor

@cmumatt cmumatt commented Jun 5, 2022

Description

Resolves issue #1044 plus others (see below).

The ShapeProps React component in docs-site previously pulled shape data from ShapeDef in @penrose/core by creating shapes on a canvas and inspecting their properties at runtime. With the Style compiler re-write, instantiating shapes requires passing a Context object. As a JS React component, ShapeProps lacked type checking and this component was never updated to work with the new compiler. This caused all shape documentation pages to crash in production.

This PR replaces the former JS React ShapeProps component with a TS component of the same name that imports a JSON file of shape properties calculated at build time rather than calling into @penrose/core to create and evaluate shapes at runtime.

This PR also fixes some additional issues that weren't previously logged as issues:

  • ColorV values were inaccurately rendered on the page.
  • PointV values were inaccurately rendered on the page.
  • The ShapeProps component would hang the UI when first accessed if the Demo component were accessed prior.
  • Docs-site build was extremely slow when both the ShapeProps and Demo components were active.
  • The JS ShapeProps component lacked any type checking, which lead to issue Shape documentation pages are crashing #1044, and others.
  • The ShapeProps component would not compile on the server side; previously a workaround was in place to force it to compile in the browser at runtime. This workaround is no longer necessary.

Implementation strategy and design decisions

  • This PR creates a new automator command, shapedefs, which outputs a JSON file of all shapes and their properties, including which properties are sampled and which properties are defaulted.
  • The ShapeProps component imports this data during build to render the shape properties pages server side.
  • This eliminates the need to load @penrose/core and instantiate shapes dynamically at runtime within ShapeProps.
  • The JS ShapeProps component is converted to TypeScript to allow type-checking. Type checking would have prevented the original issue Shape documentation pages are crashing #1044 and others.

Checklist

  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new ESLint warnings
  • I have reviewed any generated changes to the diagrams/ folder

Open questions

  • None

@cmumatt cmumatt changed the title Update shape component to work w/new style context fix: Update shape component to work w/new style context Jun 5, 2022
@codecov
Copy link

codecov bot commented Jun 5, 2022

Codecov Report

Merging #1045 (2999ccc) into main (c5220b4) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1045   +/-   ##
=======================================
  Coverage   63.48%   63.48%           
=======================================
  Files          59       59           
  Lines        7115     7115           
  Branches     1583     1583           
=======================================
  Hits         4517     4517           
  Misses       2502     2502           
  Partials       96       96           
Impacted Files Coverage Δ
packages/core/src/index.ts 62.68% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c5220b4...2999ccc. Read the comment docs.

@cmumatt cmumatt linked an issue Jun 5, 2022 that may be closed by this pull request
@cloudflare-pages
Copy link

cloudflare-pages bot commented Jun 5, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 2999ccc
Status: ✅  Deploy successful!
Preview URL: https://932d72fc.penrose-72l.pages.dev

View logs

@cmumatt cmumatt self-assigned this Jun 5, 2022
@cmumatt cmumatt requested a review from joshsunshine June 6, 2022 23:04
@cmumatt cmumatt changed the title fix: Update shape component to work w/new style context fix: Docs-site Shape Property page errors and crashes Jun 7, 2022
@cmumatt cmumatt marked this pull request as ready for review June 7, 2022 15:01
packages/automator/index.tsx Show resolved Hide resolved
@joshsunshine
Copy link
Member

Looks good to me. I think it's ready to merge.

@cmumatt cmumatt merged commit 880d197 into main Jun 7, 2022
@cmumatt cmumatt deleted the fix-1044 branch June 7, 2022 19:40
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.

Shape documentation pages are crashing
2 participants