Playground and development environment fixes for Lerna #1642

epicfaace · 2020-03-10T18:37:30Z

Reasons for making this change

Building off of work done in #1606 and #1641.

Update build and dev process and documentation (as discussed in Troubles with the new Playground #1630 (comment)). The developer will just need to run lerna bootstrap, run npm start from the package they are modifying, then run the playground from packages/playground to get a live-reloading playground.
Fix deploy previews by configuring netlify so that it runs lerna bootstrap, thus linking the packages with the local changes in the PR, before building the playground.
Export typings from @rjsf/core (fixes Export typings from @rjsf/core #1583) -- did this in this PR so that @rjsf/material-ui would work as well.
Fix playground issues in which the material-ui playground did not use the right ObjectFieldTemplate or ArrayFieldTemplate.

Fixes #1630 .

Checklist

I'm updating documentation
- I've checked the rendering of the Markdown text I've added
I'm adding or updating code
- I've added and/or updated tests
- I've updated docs if needed
I'm adding a new feature
- I've updated the playground with an example use of the feature

…eact-jsonschema-form

…jsf/material-ui

Saulzi

Have not tried this out but many of the changes are similar to what I have done, please note that the CI still seems to be broken and I have a query with regards to UMD support

Saulzi · 2020-03-10T22:03:17Z

packages/core/package.json

+    "build": "npm run build:cjs && npm run build:es",
+    "build:cjs": "NODE_ENV=production BABEL_ENV=cjs babel --extensions \".js,.jsx,.ts,.tsx\" ./src --out-dir ./dist/cjs",
+    "build:es": "NODE_ENV=production BABEL_ENV=es babel --extensions \".js,.jsx,.ts,.tsx\" ./src --out-dir ./dist/es",


Why no umd, is there still a umd build? as this is what I am using! basically I include the whole lib and allow other plugins to use version pulled in via umd with requirejs, potentually umd works with both commonjs loaders, iife i.e. the browser straight and AMD loaders

Not sure, any thoughts @chanceaclark ?

I think we might have to change build tools then. babel has a plugin for converting to UMD but it's kind of a all or nothing approach.

@Saulzi, if you mention umd works with commonjs (cjs) loaders, couldn't you just load in the cjs modules?

In the meantime, play around with using rollup and try to push a commit.

Yes it works with both commonjs and AMD loaders please refer to https://www.davidbcalhoun.com/2014/what-is-amd-commonjs-and-umd/
https://riptutorial.com/javascript/example/16339/universal-module-definition--umd-
https://www.google.com/search?q=common+javascript+modules&oq=common+javascript+modules&aqs=chrome..69i57.11167j0j7&sourceid=chrome&ie=UTF-8

AMD has been around for some time before the ES6 (ESM) standard.. i.e. dojo framework had amd support in around since 2011, many javascript developers have used things such as jquery etc which have used this pattern for some time and not even realised.. for example react builds to a umd module other popular javascript frameworks such as leaflet, moment, knockout and chart.js are built to umd modules.

@chanceaclark @epicfaace from experience rollup will make porting to typescript much easier as it is quite easy to build a and export ESM / UMD with rollup and typescript (whack it through the terser plugin for release builds) ,

Do you plan on just typescripts JSX/ES5 support rather than the current babel configuration. one thing to note is that if you plan to use babel to transpile typescript, this is bit behind the current typescript release for example it does not support newer syntax such as import type {} from "module" in ts.

In my mind webpack and browserify should be taken out back and shot (have you ever seen the intro to stealjs video), crazy amount of configuration etc and the whole node_modules black hole. I will say that steal is good for sites but not libs and another potential is parcel although I think this is aimed more at applications rather than libs)

I've updated it to use the old webpack config to create the UMD build again. We can definitely switch to rollup but for now I'm just adding the old webpack config which works and won't affect local development.

epicfaace · 2020-03-12T01:48:58Z

packages/core/package.json

+    "build": "npm run build:cjs && npm run build:es && npm run build:es:lib",
+    "build:cjs": "NODE_ENV=production BABEL_ENV=cjs babel --extensions \".js,.jsx,.ts,.tsx\" ./src --out-dir ./dist/cjs",
+    "build:es": "NODE_ENV=production BABEL_ENV=es babel --extensions \".js,.jsx,.ts,.tsx\" ./src --out-dir ./dist/es",
+    "build:es:lib": "NODE_ENV=production BABEL_ENV=es babel --extensions \".js,.jsx,.ts,.tsx\" ./src --out-dir ./lib",


@chanceaclark I had to add this line in because we needed to import things like @rjsf/core/lib/utils

I will check that I still get a UMD Module, this is vital for me and perhaps other users... essentially I pull this from the build output

@epicfaace I'm guessing that's to keep support for it. What's to stop people from using doing this now:

import { deepEquals } from '@rjsf/core/dist/utils';`

IMO, it makes more sense to export the utils along with package or creating a utils package if you want to separate it.

Also, you are probably aware, but keep in mind this is still part of the alpha major version so there is more freedom in introducing breaking changes.

Yeah, good point. It seems like react-jsonschema-form/lib/utils is already used, though, see material-ui (https://github.com/rjsf-team/react-jsonschema-form/pull/1642/files#diff-92ef0ee17db18857941b12442a459de0R6) or #1643

Additionally, the documentation even has an example of using an actual component from the lib directory (https://github.com/rjsf-team/react-jsonschema-form/pull/1642/files#diff-443373237a67e0fe395c3da27e3595ebR553).

import SchemaField from "@rjsf/core/lib/components/fields/SchemaField";

This is probably bad, as we probably don't want to constrain ourselves with file structure. However, it would also be useful for users to be able to access the default fields / widgets. Perhaps it would be best to just make a @rjsf/utils package and expose fields through @rjsf/core/fields and widgets through @rjsf/core/widgets?

I say we break out the utils package since it'll be better to maintain that way. If we need to make a path or minor to the utils package, in theory it shouldn't break anyones installs if they are using a ^ (i.e. "@rjsf/utils": "^X.X.X").

For the fields and widgets, they should probably just be exposed in the root index file still so you can import them like so:

import { ArrayField } from '@rjsf/core';`

We would just need to ensure tree shaking is set up correctly, that way they don't import everything from @rjsf/core.

Yeah, sounds good. Let's do that in another issue / PR.

epicfaace · 2020-03-12T01:50:09Z

this is now ready for review and CI is passing - @erunion @chanceaclark @Saulzi

Saulzi

This does not build on windows NODE_ENV issues.

Please see my other comments,

@epicfaace you are soooo close to the prize

Saulzi · 2020-03-12T09:29:16Z

netlify.toml

-  command = "npm run build:playground"
+  base = ""
+  publish = "packages/playground/build"
+  command = "npm run bootstrap && (npm run build || { sleep 120; false; })"


Is the sleep still needed here?

Saulzi · 2020-03-12T09:31:19Z

packages/core/package.json

+    "build": "npm run build:cjs && npm run build:es && npm run build:es:lib",
+    "build:cjs": "NODE_ENV=production BABEL_ENV=cjs babel --extensions \".js,.jsx,.ts,.tsx\" ./src --out-dir ./dist/cjs",
+    "build:es": "NODE_ENV=production BABEL_ENV=es babel --extensions \".js,.jsx,.ts,.tsx\" ./src --out-dir ./dist/es",
+    "build:es:lib": "NODE_ENV=production BABEL_ENV=es babel --extensions \".js,.jsx,.ts,.tsx\" ./src --out-dir ./lib",


I will check that I still get a UMD Module, this is vital for me and perhaps other users... essentially I pull this from the build output

packages/core/package.json

Saulzi · 2020-03-12T09:35:38Z

packages/playground/src/index.js

-function deepEquals(a, b, ca = [], cb = []) {
-  // Partially extracted from node-deeper and adapted to exclude comparison
-  // checks for functions.
-  // https://github.com/othiym23/node-deeper
-  if (a === b) {
-    return true;
-  } else if (typeof a === "function" || typeof b === "function") {
-    // Assume all functions are equivalent
-    // see https://github.com/mozilla-services/react-jsonschema-form/issues/255
-    return true;
-  } else if (typeof a !== "object" || typeof b !== "object") {
-    return false;
-  } else if (a === null || b === null) {
-    return false;
-  } else if (a instanceof Date && b instanceof Date) {
-    return a.getTime() === b.getTime();
-  } else if (a instanceof RegExp && b instanceof RegExp) {
-    return (
-      a.source === b.source &&
-      a.global === b.global &&
-      a.multiline === b.multiline &&
-      a.lastIndex === b.lastIndex &&
-      a.ignoreCase === b.ignoreCase
-    );


are some of these functions common i.e. do we have deep equality in the lib etc?

happy but DRY?

I've pointed out that there is a bug in this: #1438 (comment)

We still need to support this function until we can really refactor using an external lib. My recommendation is to convert everything to TS before we can tackle this.

Saulzi · 2020-03-12T09:50:17Z

packages/core/package.json

+    "build:cjs": "NODE_ENV=production BABEL_ENV=cjs babel --extensions \".js,.jsx,.ts,.tsx\" ./src --out-dir ./dist/cjs",
+    "build:es": "NODE_ENV=production BABEL_ENV=es babel --extensions \".js,.jsx,.ts,.tsx\" ./src --out-dir ./dist/es",
+    "build:es:lib": "NODE_ENV=production BABEL_ENV=es babel --extensions \".js,.jsx,.ts,.tsx\" ./src --out-dir ./lib",


The NODE_ENV stuff here seems to not work on windows

please see my other comments relating to UMD builds here, as this currently won't build for me I cannot check but basically need output as per UMD (this is commonjs iffe and amd all in one)

i.e..

(function (root, factory) { if (typeof define === 'function' && define.amd) { // AMD. Register as an anonymous module. define(['exports', 'b'], factory); } else if (typeof exports === 'object' && typeof exports.nodeName !== 'string') { // CommonJS factory(exports, require('b')); } else { // Browser globals factory((root.commonJsStrict = {}), root.b); } }(this, function (exports, b) { //use b in some fashion. // attach properties to the exports object to define // the exported module properties. exports.action = function () {}; }));

this basically needs cross-env to be stuck in front in package.json's.. already there in other places.

Please note that this change in package.json to build both commonjs and esm really breaks things for me, basically there was one output before which was an amd module i.e. in the dist folder which I would dump with all my other js files

note this now dumps lots of small files and not a single minified / bundled file

@Saulzi can you share some example code that relies on the UMD build? That way I can make sure that the way it outputs will work with that code.

this basically needs cross-env to be stuck in front in package.json's.. already there in other places.

Done

@Saulzi can you share some example code that relies on the UMD build? That way I can make sure that the way it outputs will work with that code.

@epicfaace @chanceaclark

https://github.com/Saulzi/react-jsonschema-form/tree/amd-example
Here is an example of using react-jsonschema-form via UMD. using all three legacy module loader types across node, and the browser.

I found a few things i.e. synthetic defaults etc with the current build...

Why build commonjs when UMD does it all for legacy module loaders.. essentially newer build tools such as rollup / webpack use ESM, node is stuck using commonjs and browser can essentually do amd (via steal / require / dojo / curl?), commonjs (stealjs), umd script tags and esm modules..

things would be so much better if react was built to esm module because then it would play nicer with rollup etc and would allow me to load it via script type module rather than relying on AMD.

I am glad this commit is now in merged as its definitely an improvement but still needs a little bit of tweaking, @epicfaace does this also resolve issues with CI i.e. is the playground preview now up to date?

@Saulzi
You said "I found a few things i.e. synthetic defaults etc with the current build..." -- did you mean that you are able to use the UMD version now or are there still issues?

@epicfaace it does work but really when referencing this in the browser and via amd you shouldn't have to use .default... it is basically because of synthetic exports.

Saulzi · 2020-03-13T00:04:05Z

packages/core/package.json

@@ -3,32 +3,34 @@
  "version": "2.0.0-alpha.2",
  "description": "A simple React component capable of building HTML forms out of a JSON schema.",
  "scripts": {
-    "build:lib": "rimraf lib && cross-env NODE_ENV=production babel -d lib/ src/",
-    "build:dist": "rimraf dist && cross-env NODE_ENV=production webpack --config webpack.config.dist.js",


This line used to cause a umd module

…ema-form into playground-fix-2

epicfaace · 2020-03-14T21:14:58Z

ready for additional review @chanceaclark @Saulzi

chanceaclark · 2020-03-16T14:05:16Z

packages/core/package.json

      "eslint --fix",
      "prettier --write",
      "git add"
    ]
  },
-  "main": "lib/index.js",
+  "main": "dist/cjs/index.js",
+  "module": "dist/es/index.js",


Probably want to add this instead of including it in the files array.

Suggested change

"module": "dist/es/index.js",

"module": "dist/es/index.js",

"typings": "index.d.ts"

chanceaclark · 2020-03-16T14:05:27Z

packages/core/package.json

  "files": [
    "dist",
-    "lib"
+    "index.d.ts"


Remove from above.

chanceaclark · 2020-03-16T14:06:01Z

packages/core/package.json

+    "build": "npm run dist:build:umd && npm run build:cjs && npm run build:es && npm run dist:build:es:lib",
+    "build:cjs": "cross-env NODE_ENV=production BABEL_ENV=cjs babel --extensions \".js,.jsx,.ts,.tsx\" ./src --out-dir ./dist/cjs",
+    "build:es": "cross-env NODE_ENV=production BABEL_ENV=es babel --extensions \".js,.jsx,.ts,.tsx\" ./src --out-dir ./dist/es",
+    "dist:build:es:lib": "cross-env NODE_ENV=production BABEL_ENV=es babel --extensions \".js,.jsx,.ts,.tsx\" ./src --out-dir ./lib",
+    "dist:build:umd": "rimraf dist && cross-env NODE_ENV=production BABEL_ENV=umd webpack --config webpack.config.dist.js",


Yikes. But we'll figure it out.

chanceaclark · 2020-03-16T14:08:10Z

package.json

@@ -1,5 +1,5 @@
 {
-  "name": "react-jsonschema-form",
+  "name": "@rjsf/core",


You probably don't want to change this. It should be the name of the repo if it's a monorepo.

Suggested change

"name": "@rjsf/core",

"name": "react-jsonschema-form",

What do you think should be done with the version attribute? I don't think Lerna automatically bumped it upon release, and I'm not sure if version is even relevant in the top of the monorepo.

chanceaclark · 2020-03-16T14:11:44Z

packages/material-ui/src/SelectWidget/SelectWidget.tsx

@@ -5,8 +5,8 @@ import MenuItem from '@material-ui/core/MenuItem';
 import Select from '@material-ui/core/Select';
 import InputLabel from '@material-ui/core/InputLabel';

-// import { WidgetProps } from 'react-jsonschema-form';
-import { asNumber, guessType } from 'react-jsonschema-form/lib/utils';
+// import { WidgetProps } from '@rjsf/core';


May want to remove this comment

chanceaclark

LGTM!

epicfaace · 2020-03-16T14:38:44Z

I wonder whether it might be better to move playground from packages/playground to the root-level directory (playground) -- because I don't think we'll ever be publishing it as a package?

edit: Actually, I think it's fine -- keeping it under packages makes the root directory's package.json cleaner so you don't necessarily have to install playground dependencies just to get started. And it's already a private package so it won't be published.

chanceaclark · 2020-03-16T14:41:01Z

I wonder whether it might be better to move playground from packages/playground to the root-level directory (playground) -- because I don't think we'll ever be publishing it as a package?

You won't be able to link @rjsf/core and the material-ui packages if we do. As long as there is a "private": true key/value in the package.json, it won't get published at all.

…nto playground-fix-2

chanceaclark and others added 4 commits February 26, 2020 10:50

feat: update build and dev process

6b64ea3

Merge branch 'update-playground' of https://github.com/chanceaclark/r…

47a4169

…eact-jsonschema-form

rename react-jsonschema-form to @rjsf/core and rjsf-material-ui to @r…

ed7efec

…jsf/material-ui

update contributing docs with new setup

ff2e678

epicfaace mentioned this pull request Mar 10, 2020

feat: update build and dev process #1606

Closed

7 tasks

epicfaace added 9 commits March 10, 2020 19:02

fix core package setup

a11970c

update netlify config to properly publish playground

5f4e945

run bootstrap on CI

68194d5

add types to @rjsf/core and make @rjsf/material-ui work

0b25167

playground: fix arrays / objects not rendering properly on material ui

bce81b4

update mui readme

f2d03e2

update docs

cd57fa3

fix netlify config

029900d

ci: install npm deps such as lerna

a9475b0

epicfaace changed the title ~~Playground fix 2~~ Playground and development environment fixes Mar 10, 2020

epicfaace changed the title ~~Playground and development environment fixes~~ Playground and development environment fixes for Lerna Mar 10, 2020

epicfaace added 2 commits March 10, 2020 21:27

switch azure pipelines order

e49cdd8

fix netlify

b3652cd

Saulzi reviewed Mar 10, 2020

View reviewed changes

epicfaace added 6 commits March 10, 2020 22:46

add --no-bail and --stream

74746cb

debug netlify build

d1d283f

update playground build

bcd06b6

fix playground prod build

3d741ce

update webpack configs for playground

c20de18

build lib file so it can be properly imported

9798a84

epicfaace commented Mar 12, 2020

View reviewed changes

fix resolve for webpack prod config

263ac41

Saulzi suggested changes Mar 12, 2020

View reviewed changes

Saulzi reviewed Mar 13, 2020

View reviewed changes

epicfaace added 5 commits March 14, 2020 16:40

remove sleep

3d87b4d

Add umd build, don't run build:es:lib in dev mode

2aa9509

Merge branch 'playground-fix-2' of github.com:rjsf-team/react-jsonsch…

83e8fc0

…ema-form into playground-fix-2

Add cross-env for windows compatibility

28739d7

fix babel umd config

90b1149

This was referenced Mar 14, 2020

Add a Chakra UI theme #1640

Closed

Create separate @rjsf/utils package #1655

Closed

chanceaclark reviewed Mar 16, 2020

View reviewed changes

epicfaace added 4 commits March 16, 2020 14:30

add WidgetProps

b19c560

fix export typings

0f96efb

fix package name

592f093

update doc

4124bb1

chanceaclark approved these changes Mar 16, 2020

View reviewed changes

epicfaace added 2 commits March 16, 2020 14:43

Merge branch 'master' of github.com:rjsf-team/react-jsonschema-form i…

9052547

…nto playground-fix-2

add boolean to widgetProps

a4a2426

epicfaace merged commit ad55e4f into master Mar 16, 2020

epicfaace deleted the playground-fix-2 branch March 16, 2020 15:17

epicfaace mentioned this pull request Mar 19, 2020

Ship untranspiled version of library #1659

Closed

3 tasks

tugglecore mentioned this pull request Mar 20, 2020

Warning: Failed prop type: Invalid prop value supplied to TextWidget. #1559

Closed

epicfaace added this to Done in V2 Mar 28, 2020

epicfaace mentioned this pull request Apr 25, 2020

Again add subthemes, fix blurring for new playground (redoing of #1623) #1723

Merged

	"module": "dist/es/index.js",
	"module": "dist/es/index.js",
	"typings": "index.d.ts"

Playground and development environment fixes for Lerna #1642

Playground and development environment fixes for Lerna #1642

Conversation

epicfaace commented Mar 10, 2020 • edited

Reasons for making this change

Checklist

Saulzi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Saulzi Mar 12, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chanceaclark Mar 12, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

epicfaace commented Mar 12, 2020

Saulzi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

epicfaace commented Mar 14, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chanceaclark Mar 16, 2020 • edited

Choose a reason for hiding this comment

chanceaclark left a comment

Choose a reason for hiding this comment

epicfaace commented Mar 16, 2020 • edited

chanceaclark commented Mar 16, 2020

epicfaace commented Mar 10, 2020 •

edited

Saulzi Mar 12, 2020 •

edited

chanceaclark Mar 12, 2020 •

edited

chanceaclark Mar 16, 2020 •

edited

epicfaace commented Mar 16, 2020 •

edited