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

Monorepo with Lerna #1501

Merged
merged 25 commits into from
Nov 10, 2019
Merged

Monorepo with Lerna #1501

merged 25 commits into from
Nov 10, 2019

Conversation

MadelineRitchie
Copy link

@MadelineRitchie MadelineRitchie commented Nov 4, 2019

Reasons for making this change

Use Lerna to maintain core and themes as a monorepo. This should streamline creation of new themes and maintenance of existing theme(s).

Status

I've published the packages to a temporary npm org using 'lerna publish'. I've also verified travis pipeline runs.

https://travis-ci.org/JasonRitchie/react-jsonschema-form/builds/607277676

https://www.npmjs.com/~jasonritchie

Tasks before finalizing PR

  • finalize package names in /packages/**/package.json files
  • finalize package version #s in /packages/**/package.json files and lerna.json
  • verify official travis pipeline works
  • verify official publish to npm works

Future opportunities for improvement

  • Expand playground with a theme chooser
  • Hoist more packages to the root of the monorepo
  • Standardize build and deployment processes across packages
  • Leverage travis to test themes as well as core

Jason Ritchie added 2 commits November 1, 2019 14:15
using @rjsf-jasonritchie org and travis to get things
fully working before creating PR
@epicfaace
Copy link
Member

Hey, will take a look at this soon, but thanks for going ahead and doing this!

SuriGill and others added 4 commits November 8, 2019 17:12
* fix: update arrays correctly when changing index

* fix: Update anyOf schema to correctly update items in an array

* fix: update schema to re-render when idschema changes
* Inject defaults in arrays

When using dependencies in array items, defaults are now injected.
This adjusts injecting arrays into the form data from defaults to be
in line with the previous behaviour. So the defaults are injected when
there is either no value defined in the form data or when an array is
already set in the form data, injects only into existing entries.
This makes sure that we don't change any existing behaviour by creating
array entries where we did not create them before.

For example for following schema:
{
  "type": "array",
  "minItems": 2,
  "items": {
    "title": "asd",
    "type": "object",
    "properties": {
      "item": {
        "type": "string",
        "default": "foo"
      }
    }
  }
}

and following form data:
[{}]

The result will be (note how it did not full up the array to min items)
[{ "item": "foo" }]

* Remove redundant comment

* Add documentation and example

* Merge arrays defined in parent schema by overwriting same level arrays

* fix typo

* fix typos

* wording changes

Co-Authored-By: Dominik del Bondio <dominik.del.bondio@bitextender.com>
Copy link
Member

@epicfaace epicfaace left a comment

Choose a reason for hiding this comment

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

I think it would be best to create a separate directory for playground (at the same level as packages) because we would want to have a single playground that showcases many themes. This would end up merging the playgrounds for core and material-ui eventually, though you could start with just using the core playground for now.

@@ -0,0 +1,131 @@
# Created by https://www.gitignore.io/api/osx,node,linux,windows
Copy link
Member

Choose a reason for hiding this comment

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

Why create separate gitignores for each package (as opposed to having a single gitignore at the root of the repo)?

Copy link
Author

Choose a reason for hiding this comment

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

cleaned up the redundant gitignore files

@MadelineRitchie
Copy link
Author

I think there's a clear path of improvements we should pursue after we get this merged. There's good reason to merge this before it's "perfect" because integrating upstream changes when the paths of most files has changed has some friction and can leave confusing commit history.

I'll add "themes in playground" to the "future improvements" list in the initial request.

What do you think about the "Tasks before finalizing PR" list as it stands?

@epicfaace epicfaace marked this pull request as ready for review November 10, 2019 05:45
@epicfaace
Copy link
Member

epicfaace commented Nov 10, 2019

Thanks! Good point about merging this as soon as possible.

I ended up moving the playground folder back to packages/core because it seems that moving it to a separate folder will require a new webpack configuration, etc. which can probably be done later. Waiting for the CI to pass and then will merge.

@epicfaace epicfaace changed the title Monorepo with Lerna [draft] Monorepo with Lerna Nov 10, 2019
@epicfaace epicfaace merged commit ef8b7fc into rjsf-team:master Nov 10, 2019
@MadelineRitchie MadelineRitchie deleted the lerna branch November 10, 2019 21:46
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