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

Update React dependencies and retrofit clients (IDE and automator) #388

Merged
merged 16 commits into from
Oct 23, 2020

Conversation

wodeni
Copy link
Member

@wodeni wodeni commented Oct 16, 2020

Description

Related issues: #336

Our dependencies create-react-app-ts and tslint are both deprecated. This PR moves our web version to create-react-app and eslint, which support TypeScript now.

The new TypeScript runtime also includes some breaking changes that affect both the automator and IDE. These two projects depend on penrose-web and use its API functions. This PR also aims to retrofit both modules to the new API.

Implementation strategy and design decisions

  • Due to the change from relative to absolute paths in penrose-web, the IDE fails to resolve modules of it. I used ttsc (tsc with transformers) to transform paths in the compiled JavaScript files.
  • Migration to CRA and eslint, as well as associated version bumps
  • Additional configuration file for build-lib to bypass CRA limitations (e.g. noEmit).
  • Add tslog for logging

Examples with steps to reproduce them

  • Reinstall both penrose-web and the IDE by following the wiki
  • Repro: npx ts-node ./index.tsx "batch" "substanceLibrary.json" "styleLibrary.json" "elementLibrary.json" "out"

Open questions

Questions that require more discussion or to be addressed in future development:

…se ttsc (tsc with transformers) for build-lib
@kai-qu
Copy link
Contributor

kai-qu commented Oct 16, 2020

Thanks for doing this! I took a quick look but am not super familiar with ts internals so can't say much about that. Is this ready to merge?

Also, +1 to Rename react-renderer: I'm thinking penrose-web

@wodeni
Copy link
Member Author

wodeni commented Oct 16, 2020

Thanks for doing this! I took a quick look but am not super familiar with ts internals so can't say much about that. Is this ready to merge?

Also, +1 to Rename react-renderer: I'm thinking penrose-web

Still need to address some issues listed in the open questions section. The timeline depends on how soon we want the automator and IDE back.

@wodeni
Copy link
Member Author

wodeni commented Oct 21, 2020

Ported automator. Only two examples in the substanceLibrary.json work with the new optimizer. I renamed the old registry to substanceLibrary-old.json. Fresh out of the pipeline:
image
image

Repro: npx ts-node ./index.tsx "batch" "substanceLibrary.json" "styleLibrary.json" "elementLibrary.json" "out"

  • I ended up keeping the timers around, so we should have the same metadata for profiling later.
  • automator now links to penrose-web, instead of importing it directly. This is mostly due to the new changes to module paths in penrose-web and took a bit of trial-and-error to get it right.
  • Therefore, I changed the circleCI script to add the link step into the pipeline. All tests passed now.
  • @hypotext can you update venn.sty (and any other Style files that can be easily fixed)?

@wodeni wodeni merged commit ca62915 into master Oct 23, 2020
@wodeni wodeni deleted the react-infra branch October 23, 2020 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants