feat(common): pass manifest json to sandbox #273

demetriusfeijoo · 2023-09-18T11:45:59Z

What?

It allows the Sandbox/Container to bootstrap options from a manifest file located within a field-plugin root folder.

Why?

JIRA: EXT-1877

see the RFC

When our Vite printDev plugin finds the manifest file:

When our Vite printDev plugin doesn't find the manifest file:

When our Vite printDev plugin finds the manifest file but it's not valid:

How to test? (optional)

From the root folder of this repository, run:

yarn dev:container
yarn dev:react
Open Sandbox

It should load the options automatically (from the manifest file) like below:

If you want to change the manifest options locally, edit this file here React Manifest

vercel · 2023-09-18T11:46:03Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
plugin-sandbox	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 22, 2023 10:15am

packages/cli/templates/js/field-plugin.config.json

packages/container/src/components/FieldPluginContainer.tsx

BibiSebi · 2023-09-18T13:09:15Z

packages/field-plugin/helpers/vite/src/utils/text.ts

@@ -0,0 +1,12 @@
+const styles = {
+  reset: '\u001b[0m',


There is also a library for having colorful outputs in your console logs. I believe it is this one (https://www.npmjs.com/package/chalk) we are also using in the CLI

I'll check it out @BibiSebi. I saw the CLI uses it but have never tried it on my own. 🤔

Yeah tbh I'd prefer using the library instead of keeping the color codes here. And it's not increasing our bundle size either (only for development mode).

But if you add chalk, you should add it to the vite config's externals as well. (Let me know if you don't understand what that is for)

BibiSebi · 2023-09-18T13:12:37Z

Side note: just randomly found this library that is even used by prettier https://github.com/cosmiconfig/cosmiconfig that helps loading configurations. Maybe we can check it out

packages/field-plugin/helpers/vite/src/sandbox.ts

BibiSebi · 2023-09-18T13:21:20Z

packages/field-plugin/helpers/vite/src/manifest.ts

+
+export const load = (): Manifest => {
+  try {
+    const content: string = readFileSync(MANIFEST_FULL_PATH, {


maybe here we can do some kind of validation ? to check if the schema is ok ? it might be that the user types in something weird. What do you think?

It would be nice Bibi. You mean using some library such as joi, right?

It'd be a great improvement o/

Thanks for the suggestion @BibiSebi. 🚀

eunjae-lee · 2023-09-18T15:23:34Z

Side note: just randomly found this library that is even used by prettier https://github.com/cosmiconfig/cosmiconfig that helps loading configurations. Maybe we can check it out

Interesting. It's similar to https://github.com/unjs/c12

I guess those packages are specifically useful when we want to support different types of configs (e.g. an object within package.json, something.json, something.js, something.ts, ...)

demetriusfeijoo · 2023-09-18T15:29:51Z

Side note: just randomly found this library that is even used by prettier https://github.com/cosmiconfig/cosmiconfig that helps loading configurations. Maybe we can check it out

Thanks for sharing @BibiSebi.
I'm going to read about it and make some tests here. 🙌

eunjae-lee · 2023-09-18T15:27:53Z

packages/cli/templates/js/field-plugin.config.json

@@ -0,0 +1,12 @@
+{


I think this will lead to users having unexpected options (option1, option2) by default (probably without knowing it).

What if we have an empty array for options in the templates?

Makes sense @eunjae-lee.

It should be enough once we work on the documentation.

Then could you remove these options from this branch? I know it might be annoying for you to test with actual values, but they shouldn't be committed here any more.

Done @eunjae-lee.

packages/cli/templates/js/field-plugin.config.json

eunjae-lee · 2023-09-18T15:48:23Z

packages/field-plugin/helpers/vite/src/manifest.ts

+export const load = (): Manifest => {
+  try {
+    const content: string = readFileSync(MANIFEST_FULL_PATH, {
+      encoding: 'utf-8',


I think we can drop this as it's by default.

You mean without making use of other libraries such as cosmiconfig, right?

Oh I meant "utf-8" here

I've tried without informing the encoding as suggested, @eunjae-lee, but when it's not informed, the readFileSync returns a buffer instead of a string. As I'd need to convert it into a string in order to call JSON.parse, I think the easiest way would be to keep the encoding part but in its shortcut way:

const content: string = readFileSync(MANIFEST_FULL_PATH, 'utf-8')

I'll commit this way and if you may have another option, let me know, please.

packages/field-plugin/helpers/vite/src/sandbox.ts

eunjae-lee · 2023-09-18T15:56:40Z

packages/field-plugin/helpers/vite/src/utils/text.ts

@@ -0,0 +1,12 @@
+const styles = {
+  reset: '\u001b[0m',


Yeah tbh I'd prefer using the library instead of keeping the color codes here. And it's not increasing our bundle size either (only for development mode).

But if you add chalk, you should add it to the vite config's externals as well. (Let me know if you don't understand what that is for)

…toryblok/field-plugin into EXT-1877-pass-manifest-json-to-sandbox

packages/field-plugin/helpers/vite/src/manifest.ts

eunjae-lee · 2023-09-21T15:54:20Z

ping me here when you're ready for another review :)

Co-authored-by: Eunjae Lee <hey@eunjae.dev>

…toryblok/field-plugin into EXT-1877-pass-manifest-json-to-sandbox

demetriusfeijoo · 2023-09-21T18:39:16Z

packages/cli/templates/js/field-plugin.config.json

@@ -0,0 +1,12 @@
+{


Done @eunjae-lee.

demetriusfeijoo · 2023-09-21T18:40:52Z

packages/field-plugin/helpers/vite/src/manifest.ts

+  }
+}
+
+const validateSchema = (manifest: Manifest): void => {


I've added this simple validation, without using any external packages, here since we decided not to go too far on this.

demetriusfeijoo · 2023-09-21T18:42:01Z

packages/field-plugin/helpers/vite/src/manifest.ts

+
+export const load = (): Manifest => {
+  try {
+    const content: string = readFileSync(MANIFEST_FULL_PATH, 'utf8')


Here I kept informing the encoding since otherwise it would return me a Buffer instead of a string.

So, this way is the simplest one, I guess. If there is any other better way, let me know, please.

We've had some other code like readFileSync(...).toString(). I don't know what's better, though.

demetriusfeijoo · 2023-09-21T18:47:56Z

packages/field-plugin/helpers/vite/src/utils/text.ts

+
+const chalk = new Chalk({ level: 1 })
+
+export const green = (text: string) => chalk.green(text)


I've decided to keep this file here, even after using the Chalk package, as it serves as an abstract layer and allows us to easily switch between libraries in the future (e.g. kleur). Such as it was done now. I've just needed to change it here and for the rest of the helper was transparent.

demetriusfeijoo · 2023-09-21T19:17:17Z

It's ready for another review round @eunjae-lee. 🙌

eunjae-lee · 2023-09-22T08:35:04Z

packages/field-plugin/helpers/vite/src/manifest.ts

+    throw new Error(`When declared, the 'options' property should be an array`)
+  }
+
+  manifest.options?.forEach((o) => {


let's rename this o to option to be more informative. Other than that, it looks good!

Thanks for the suggestion here, @eunjae-lee. 🙌

I've just renamed it.

If there is something else, let me know, please.

eunjae-lee

Looks good to me @demetriusfeijoo !

Could you also create another PR to add some tests? I should've noticed it earlier but now I think we could add some tests for some functions in manifest.ts and sandbox.ts. However I don't want to delay the merge of this PR. So let's do it separately :)

demetriusfeijoo · 2023-09-22T13:15:16Z

Thanks for the effort in reviewing it @eunjae-lee.

I've created a task for adding the tests, thanks for pointing it out. 🙌

demetriusfeijoo added 3 commits September 18, 2023 08:23

add manifest example in our templates

09ebd05

read manifest file from templates

561464f

load manifest options from query-string

d7c9b21

demetriusfeijoo commented Sep 18, 2023

View reviewed changes

packages/cli/templates/js/field-plugin.config.json Outdated Show resolved Hide resolved

demetriusfeijoo commented Sep 18, 2023

View reviewed changes

packages/container/src/components/FieldPluginContainer.tsx Show resolved Hide resolved

adjust tests

6e131af

vercel bot deployed to Preview September 18, 2023 12:48 View deployment

eunjae-lee changed the title ~~feat(common): Pass manifest json to sandbox~~ feat(common): pass manifest json to sandbox Sep 18, 2023

demetriusfeijoo requested review from eunjae-lee and BibiSebi September 18, 2023 12:54

demetriusfeijoo marked this pull request as ready for review September 18, 2023 12:54

BibiSebi reviewed Sep 18, 2023

View reviewed changes

packages/field-plugin/helpers/vite/src/sandbox.ts Outdated Show resolved Hide resolved

BibiSebi reviewed Sep 18, 2023

View reviewed changes

packages/field-plugin/helpers/vite/src/sandbox.ts Outdated Show resolved Hide resolved

BibiSebi reviewed Sep 18, 2023

View reviewed changes

eunjae-lee reviewed Sep 18, 2023

View reviewed changes

demetriusfeijoo added 3 commits September 18, 2023 15:15

rename function for checking the existence of the manifest

122dc16

remove unnecessary method

f2f3e38

remove unnecessary use o type 'any'

ca9a4c6

vercel bot deployed to Preview September 18, 2023 18:22 View deployment

add manifest parameter only if necessary

6f19373

vercel bot deployed to Preview September 19, 2023 16:20 View deployment

demetriusfeijoo and others added 3 commits September 21, 2023 07:52

Merge branch 'main' into EXT-1877-pass-manifest-json-to-sandbox

d5e1d10

remove all options from the manifest files

a615ae1

Merge branch 'EXT-1877-pass-manifest-json-to-sandbox' of github.com:s…

90b59de

…toryblok/field-plugin into EXT-1877-pass-manifest-json-to-sandbox

vercel bot deployed to Preview September 21, 2023 10:54 View deployment

vercel bot deployed to Preview September 21, 2023 10:57 View deployment

use chalk to style the logs

acc9176

eunjae-lee reviewed Sep 21, 2023

View reviewed changes

packages/field-plugin/helpers/vite/src/manifest.ts Outdated Show resolved Hide resolved

demetriusfeijoo and others added 2 commits September 21, 2023 14:02

Remove utf-8 explicity encoding since it's the default

80f9d32

Co-authored-by: Eunjae Lee <hey@eunjae.dev>

Merge branch 'EXT-1877-pass-manifest-json-to-sandbox' of github.com:s…

54fbea4

…toryblok/field-plugin into EXT-1877-pass-manifest-json-to-sandbox

vercel bot had a problem deploying to Preview September 21, 2023 17:04 Failure

demetriusfeijoo added 2 commits September 21, 2023 15:37

return the explicity encoding

4081224

add manifest schema validation

2319e6e

vercel bot deployed to Preview September 21, 2023 18:40 View deployment

demetriusfeijoo commented Sep 21, 2023

View reviewed changes

demetriusfeijoo requested review from eunjae-lee and BibiSebi September 21, 2023 19:16

eunjae-lee reviewed Sep 22, 2023

View reviewed changes

improve readability when validating the options

5c0439a

vercel bot deployed to Preview September 22, 2023 10:15 View deployment

demetriusfeijoo requested a review from eunjae-lee September 22, 2023 10:36

eunjae-lee approved these changes Sep 22, 2023

View reviewed changes

demetriusfeijoo merged commit d9bdb1b into main Sep 22, 2023
11 checks passed

demetriusfeijoo deleted the EXT-1877-pass-manifest-json-to-sandbox branch September 22, 2023 13:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(common): pass manifest json to sandbox #273

feat(common): pass manifest json to sandbox #273

demetriusfeijoo commented Sep 18, 2023 •

edited

Loading

vercel bot commented Sep 18, 2023 •

edited

Loading

BibiSebi Sep 18, 2023

demetriusfeijoo Sep 18, 2023

eunjae-lee Sep 18, 2023

BibiSebi commented Sep 18, 2023

BibiSebi Sep 18, 2023

demetriusfeijoo Sep 18, 2023

eunjae-lee commented Sep 18, 2023

demetriusfeijoo commented Sep 18, 2023

eunjae-lee Sep 18, 2023

demetriusfeijoo Sep 18, 2023 •

edited

Loading

eunjae-lee Sep 20, 2023

demetriusfeijoo Sep 21, 2023

eunjae-lee Sep 18, 2023

demetriusfeijoo Sep 18, 2023

eunjae-lee Sep 19, 2023

demetriusfeijoo Sep 21, 2023

eunjae-lee Sep 18, 2023

eunjae-lee commented Sep 21, 2023

demetriusfeijoo Sep 21, 2023

demetriusfeijoo Sep 21, 2023

demetriusfeijoo Sep 21, 2023

eunjae-lee Sep 22, 2023

demetriusfeijoo Sep 21, 2023

demetriusfeijoo commented Sep 21, 2023

eunjae-lee Sep 22, 2023

demetriusfeijoo Sep 22, 2023

eunjae-lee left a comment

demetriusfeijoo commented Sep 22, 2023


		const chalk = new Chalk({ level: 1 })

		export const green = (text: string) => chalk.green(text)

feat(common): pass manifest json to sandbox #273

feat(common): pass manifest json to sandbox #273

Conversation

demetriusfeijoo commented Sep 18, 2023 • edited Loading

What?

Why?

How to test? (optional)

vercel bot commented Sep 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BibiSebi commented Sep 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eunjae-lee commented Sep 18, 2023

demetriusfeijoo commented Sep 18, 2023

Choose a reason for hiding this comment

demetriusfeijoo Sep 18, 2023 • edited Loading

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

eunjae-lee commented Sep 21, 2023

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

demetriusfeijoo commented Sep 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eunjae-lee left a comment

Choose a reason for hiding this comment

demetriusfeijoo commented Sep 22, 2023

demetriusfeijoo commented Sep 18, 2023 •

edited

Loading

vercel bot commented Sep 18, 2023 •

edited

Loading

demetriusfeijoo Sep 18, 2023 •

edited

Loading