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

3042: Refactor client applications into a monorepo with turborepo, extract fides-js into a standalone package, and improve privacy-center to load configuration at runtime #3105

Merged
merged 78 commits into from
May 5, 2023

Conversation

eastandwestwind
Copy link
Contributor

@eastandwestwind eastandwestwind commented Apr 19, 2023

Closes #3042

❗ Potential area of risk in deployment- https://github.com/ethyca/fides/pull/3105/files/faab9e7d25dc92c4463b2d3facde7a1858cc64ac#diff-0f3c0f74b23a0b3e4798f66a9b7ffd7f5d850165a10793f9a101235e75f1b61bR36, let's be sure to watch for this at time of deployment to ensure admin-ui is deployed appropriately to pypi

Code Changes

  • Integrate Turborepo in /clients so that we can share code between all clients.
  • Extracted out fides-consent from privacy-center into a first-class package in clients so that 1) we can share code from fides-consent between all clients, and 2) we're no longer reliant on having to build/run the privacy-center to use fides-consent.
  • Renamed fides-consent to fides-js to clarify that it can support other, non-consent functionality in the future (like proxying API calls to Fides, being used for DSR requests, etc.). See Refactor fides-consent and privacy-center to load all configuration at run-time (instead of build-time), and rename fides-consent -> fides-js #3135
  • Refactored privacy-center and fides-js to load configuration at run-time, instead of build-time. This allows editing the config.json file provided to the Privacy Center and no longer requiring a npm run build to update the changes. See Refactor fides-consent and privacy-center to load all configuration at run-time (instead of build-time), and rename fides-consent -> fides-js #3135
  • Restructured Docker builds across all environments (dev, test, prod) to accommodate the new infra to install/build/run clients together and separately.
  • Updated nox commands, gitlab ci commands, and front-end dev docs to call necessary turbo commands.
  • Change bundle output of fides-js to fides.js instead of fides.global.js
  • Remove duplicate fides-demo.html from fides-js
  • Address TODOs for fides-js: document src/fides.ts, cleanup Fides.init, etc.
  • Add (empty?) Jest test suite to fides-js
  • Fix Cypress & Jest tests for all clients
  • Rollback to Rollup for bundling the fides-js package

Follow-up issues:

Dev Changes

If you only use nox commands, continue as before, nothing needed.

If you wish to run any/all clients outside of the nox environment, with Turborepo as part of the codebase, you'll need to do 3 things:

  1. Install Turborepo globally so that you can use it on the command line npm install turbo --global
  2. In clients folder, run npm install. This will install the new dev dependency turbo, which is basically a wrapper around npm that helps manage shared dependencies. This will also install the appropriate dependencies in clients and in each client within clients folder. You may need to first remove all node_modules in each of the clients (admin-ui, pc, etc)
  3. In short, use turbo instead of npm. E.g. instead of running npm run dev, use turbo run dev, etc. All commands that existed before should still exist, given you're using turbo. Let me know if something is broken for you!

Also note that to install packages in any package in clients:

npm install <package> --workspace=<workspace>

Example:

npm install react --workspace=admin-ui

See https://turbo.build/repo/docs/handbook/package-installation#addingremovingupgrading-packages for more details

Steps to Confirm

  • All commands used to develop against the project should still work as before

Pre-Merge Checklist

@eastandwestwind eastandwestwind added the do not merge Please don't merge yet, bad things will happen if you do label Apr 19, 2023
@eastandwestwind eastandwestwind marked this pull request as draft April 19, 2023 18:45
@codecov
Copy link

codecov bot commented Apr 19, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.01 ⚠️

Comparison is base (8d9ed30) 87.37% compared to head (9149f2f) 87.36%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3105      +/-   ##
==========================================
- Coverage   87.37%   87.36%   -0.01%     
==========================================
  Files         313      313              
  Lines       18243    18248       +5     
  Branches     2366     2368       +2     
==========================================
+ Hits        15940    15943       +3     
- Misses       1871     1872       +1     
- Partials      432      433       +1     

see 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@cypress
Copy link

cypress bot commented Apr 19, 2023

Passing run #1752 ↗︎

0 4 0 0 Flakiness 0

Details:

Merge 9149f2f into 422040d...
Project: fides Commit: 0711bd8b34 ℹ️
Status: Passed Duration: 00:57 💡
Started: May 4, 2023 1:39 PM Ended: May 4, 2023 1:40 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@eastandwestwind eastandwestwind marked this pull request as ready for review April 20, 2023 17:35
Copy link
Contributor

@allisonking allisonking left a comment

Choose a reason for hiding this comment

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

woo turbo!! nice job working through setting this up!

Copy link
Contributor

@TheAndrewJackson TheAndrewJackson left a comment

Choose a reason for hiding this comment

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

Things seemed to working well! I agree with what @allisonking said. It's probably worth shipping it and ironing out the kinks this week.

I'm looking forward to using this

Copy link
Contributor

@NevilleS NevilleS left a comment

Choose a reason for hiding this comment

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

OK, I think this is ready to go. Lots of follow-up issues to tackle though, so before we merge we'll just need to figure out a safe release plan!

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.

Infrastructure to share code between all our clients- create shared components-ui
5 participants