feat(lib): export useFieldPlugin react hook #224

eunjae-lee · 2023-07-05T15:55:07Z

What?

This enables users to import useFieldPlugin directly from the library instead of embedding the code within the templates.

import { useFieldPlugin, FieldPluginProvider } from '@storyblok/field-plugin/react';

As you can see, we're importing it from the submodule of the library. Not @storyblok/field-plugin but @storyblok/field-plugin/react.

Why?

JIRA: EXT-1804

We exposed the actual implementation of useFieldPlugin inside the template for many reasons, but I think the most important part was because it wasn’t finished back then.

Now that they’re getting stable, we could include them within the library. It makes our templates cleaner. Also, users will be able to receive updates if we publish new feature or fix to those hooks.

Implementation Details

There are different ways to implement this. Let me list the approaches and explain how I chose one. From now one, let's call this react specific code "react helper".

Approach 1

I put the react helper directly inside packages/field-plugin/src folder. It was something like:

packages
  ㄴ field-plugin
    ㄴ src
      ㄴ react-helper
        ㄴ index.ts
        ㄴ useFieldPlugin.ts
        ㄴ FieldPluginProvider.tsx

And I updated yarn build to automatically copy react-helper folder into dist/ (without any bundling or transpiling).`

Then I added something like this to the package.json:

{
  "exports": {
    ".": {
      "import": "./dist/field-plugin.js",
      "require": "./dist/field-plugin.umd.cjs",
      "types": "./dist/index.d.ts"
    },
    "./react": {
      "import": "./dist/react-helper/index.ts"
    }
  },
}

Although this react submodule is exporting a typescript file index.ts, it worked well because the template is using vite and vite is able to resolve this typescript file with no problem.

However, TypeScript complained that there is no type definition for @storyblok/field-plugin/react. That's a problem .. that this approach cannot solve.

Approach 2

We could manually write a type definition for the react helper. It's not going to change much in the future, but still having to maintain it by hand might be cumbersome (we also need to support vue, ..)

What if we want to generate type definition out of the react-helper/index.ts ? It's possible, but then it means we need to have proper dev dependencies, tsconfig, vite config, etc within this storyblok/field-plugin package. It's going to be a nightmare because, what about other frameworks? This package is going to be bloated.

Approach 3

This is the one I chose in this pull-request. I created

packages
  ㄴ field-plugin
  ㄴ helper-react
  ㄴ helper-... (more to come)

This react helper package is its own library bundled with vite. However, it will be too much to maintain if we release it as @storyblok/field-plugin-react.

Instead, when building @storyblok/field-plugin, it also builds this react helper, and copies the dist of the react helper over to the dist of @storyblok/field-plugin.

# packages/field-plugin/scripts/copy-helpers.sh

cp -r ../helper-react/dist/* ./dist/react/

The main upsides about this approach are

(a) we can generate type definition with proper tooling
(b) react helper can have its own eslint config, tsconfig, etc.

In face, the previous "Approach 1" couldn't cover (b).

Conclusion

I took the approach 3, but if you have another approach to try, let me know :)

How I Added the React Helper

First of all, the folder name and package name are helper-react (not react-helper) to see all the helpers in the folder tree alphabetically. Let me know what you think.

Basically I ran this command:

cd packages/
yarn create vite

and started configuring the helper package. More details will be left as comments.

vercel · 2023-07-05T15:55:11Z

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	Jul 10, 2023 0:42am

eunjae-lee · 2023-07-06T13:29:30Z

package.json

@@ -37,6 +37,7 @@
    "build:lib": "yarn workspace @storyblok/field-plugin build",
    "build:cli": "yarn workspace @storyblok/field-plugin-cli build",
    "build:container": "yarn build:lib && yarn workspace container build",
+    "build:helpers": "yarn workspaces foreach --include \"helper-*\" run build",


Starting the package names with helper- is useful in this case.

packages/cli/templates/react/package.json

packages/cli/templates/react/tsconfig.json

packages/field-plugin/package.json

packages/field-plugin/vite.config.ts

eunjae-lee · 2023-07-06T13:41:35Z

packages/helper-react/package.json

+  "scripts": {
+    "dev": "vite",
+    "build": "tsc && vite build",
+    "lint": "eslint src --ext ts,tsx --report-unused-disable-directives --max-warnings 0 --resolve-plugins-relative-to ."


Something I've learned:

I added --resolve-plugins-relative-to . to the eslint command because eslint was traveling up to the root of this repository to gather all the eslint configs. By applying this parameter, it forces eslint to only look for the configs and plugins within this folder.

Would it be possible to set "root": true in the eslint config file? https://eslint.org/docs/latest/use/configure/configuration-files#cascading-and-hierarchy

packages/helper-react/tsconfig.json

packages/helper-react/tsconfig.node.json

packages/helper-react/vite.config.ts

eunjae-lee · 2023-07-06T13:48:02Z

packages/helper-react/vite.config.ts

+      external: ['react', 'react-dom', '@storyblok/field-plugin'],
+      output: {
+        globals: {
+          react: 'React',
+          'react-dom': 'ReactDOM',
+          '@storyblok/field-plugin': 'FieldPlugin',
+        },
+      },


Although we're depending on these 3 libraries, we're excluding them in the final bundle output.

this part is a bit unclear to me...are we excluding the dependencies because we are bundling it together with the field plugin package and therefore they are already bundled there?

I'll show you this on Wednesday. Don't forget to ask! :)

BibiSebi · 2023-07-10T07:23:00Z

This is something I have very little knowledge about, but I wonder if the tree shaking is working correctly here, like if I am using react I do not want to get the vue code also being a part of the code.

eunjae-lee · 2023-07-10T10:11:13Z

@BibiSebi Feel free to ask a ton of questions until you get comfortable enough with implementing the next one :) (We could even have a call if you want)

Anyway, because of exports in the package.json, we have multiple entries now.

@storyblok/field-plugin
@storyblok/field-plugin/react

If someone imports only from @storyblok/field-plugin, then they don't import anything from @storyblok/field-plugin/react. Even when they import something from @storyblok/field-plugin/react, the treeshaking is the same. For example,

import { useFieldPlugin } from '@storyblok/field-plugin/react';

They didn't import FieldPluginProvider from that package. Then it gets treeshaken.

BibiSebi

Thank you for your work on this one! let's merge it and on wednesday talk trough the vue part :) also thank you so much for the explanations. They were super helpful! 🙏

BibiSebi · 2023-07-10T07:18:22Z

packages/helper-react/vite.config.ts

+      external: ['react', 'react-dom', '@storyblok/field-plugin'],
+      output: {
+        globals: {
+          react: 'React',
+          'react-dom': 'ReactDOM',
+          '@storyblok/field-plugin': 'FieldPlugin',
+        },
+      },


this part is a bit unclear to me...are we excluding the dependencies because we are bundling it together with the field plugin package and therefore they are already bundled there?

johannes-lindgren

There's another approach we could take:

Write javascript files and separate typescript definition files, and annotate the JavaScript files with the @type tag with JSDoc.

In this way, we

avoid a build step
get full TypeScript support within the source code (while developing)
get full TypesScript support when importing the library
make it easy to maintain

I also wonder if the "helpers" should be part of the field-plugin package?

packages/field-plugin/helpers/react

Also, maybe "helper" is not descriptive enough. Not sure what would be a better wording though... maybe "exports"?

johannes-lindgren · 2023-07-25T09:11:29Z

packages/helper-react/src/provider.tsx

Can we rename this to FieldPluginProvider.ts?

johannes-lindgren · 2023-07-25T09:11:41Z

packages/helper-react/src/context.ts

Can we rename this to FieldPluginContext.ts?

eunjae-lee · 2023-07-25T09:39:34Z

There's another approach we could take:

Write javascript files and separate typescript definition files, and annotate the JavaScript files with the @type tag with JSDoc.

@johannes-lindgren thanks for your feedback. Actually "Approach 2" was about this, writing type definition manually (which could also be automated by jsdoc as you said). But still it comes to a problem that you have to include dev dependencies for eslint configs for react, vue, ... within the @storyblok/field-plugin package, which we wanted to avoid from the beginning 😅

eunjae-lee · 2023-07-25T11:56:05Z

I also wonder if the "helpers" should be part of the field-plugin package?

packages/field-plugin/helpers/react

Also, maybe "helper" is not descriptive enough. Not sure what would be a better wording though... maybe "exports"?

@johannes-lindgren As you initially worried about bloating the field-plugin package and as I agreed with it, I separated them as separate packages, and wrote a script to merge them during the build time.

And "helper" is obviously not my favorite part 😂 It's not visible to end-users, but we get to see this name in the codebase. Feel free to suggest any other name, since we can always rename it without introducing breaking changes to users.
To me, exports sounds too similar to export syntax in ES Modules, though.

johannes-lindgren · 2023-07-26T12:58:26Z

I also wonder if the "helpers" should be part of the field-plugin package?
packages/field-plugin/helpers/react
Also, maybe "helper" is not descriptive enough. Not sure what would be a better wording though... maybe "exports"?

@johannes-lindgren As you initially worried about bloating the field-plugin package and as I agreed with it, I separated them as separate packages, and wrote a script to merge them during the build time.

And "helper" is obviously not my favorite part 😂 It's not visible to end-users, but we get to see this name in the codebase. Feel free to suggest any other name, since we can always rename it without introducing breaking changes to users. To me, exports sounds too similar to export syntax in ES Modules, though.

@eunjae-lee Yeah exports was just an example to get the idea across...

What do you think about grouping them into a common directory such as helpers/?

Do you think it would be a good idea to move these workspaces somewhere into packages/field-plugin? Like we moved the templates into packages/cli/templates?

Here is an idea: keep each export in a separate workspace while at the same time skipping the compilation step by having type declaration files and non-compiled JavaScript files. So like approach 2, it lacks a compilation step; and like approach 3, it has multiple workspaces.

I don't have practical experience setting up a project like this, so I haven't learned the "best practices" (if there is such a thing).

eunjae-lee · 2023-07-26T13:16:52Z

@johannes-lindgren yeah we could have them under packages/field-plugin/helpers/<xyz> (or something other than helpers), if you think it seems to be a better structure.

Here is an idea: keep each export in a separate workspace while at the same time skipping the compilation step by having type declaration files and non-compiled JavaScript files. So like approach 2, it lacks a compilation step; and like approach 3, it has multiple workspaces.

I'm not sure if I understand this. What do you mean by non-compiled JavaScript files? You mean just plain JavaScript files that don't need compilation? Yes, we can do that. The only downside to this approach is that we cannot have eslint config for the specific helper if we don't have build process. Or, do you mean to have eslint configs per each helper, but just not have compilation?

johannes-lindgren · 2023-08-01T08:59:10Z

@johannes-lindgren yeah we could have them under packages/field-plugin/helpers/<xyz> (or something other than helpers), if you think it seems to be a better structure.

Here is an idea: keep each export in a separate workspace while at the same time skipping the compilation step by having type declaration files and non-compiled JavaScript files. So like approach 2, it lacks a compilation step; and like approach 3, it has multiple workspaces.

I'm not sure if I understand this. What do you mean by non-compiled JavaScript files? You mean just plain JavaScript files that don't need compilation? Yes, we can do that. The only downside to this approach is that we cannot have eslint config for the specific helper if we don't have build process. Or, do you mean to have eslint configs per each helper, but just not have compilation?

I mean the last sentence (I think):

Or, do you mean to have eslint configs per each helper, but just not have compilation?

Each helper would have its own package and there would be no compilation of the JavaScript and type definition files. Each "helper" would have its own directory, for example:

packages/field-plugin/helpers/
- react/
  - package.json
  - .eslintrc.mjs
  - src/
    - index.js
    - index.d.ts
    - (... other files, referenced by the index files)

field-plugin/package.json would include helpers in the files array, and reference the entry

Something like

{
  "files": ["helpers"],
  "exports": {
    "./react": {
      "import": "./helpers/react/src/index.js",
      "types": "./helpers/react/src/index.d.ts"
    },
}

maybe this will work:

{
  "files": ["dist", "helpers"],
  "exports": {
    "./*": {
      "import": "./helpers/*/src/index.js",
      "types": "./helpers/*/src/index.d.ts"
    },
}

## What? This adds `@storyblok/field-plugin/vue2` submodule. Related: - #228 - #224 ## Why? JIRA: EXT-1828  ## How to test? (optional)

eunjae-lee · 2023-08-01T09:56:38Z

@johannes-lindgren okay, I see your point. I mostly agree with what you said, and I wonder why you want to exclude compilation step while you keep its own eslint config, etc.

johannes-lindgren · 2023-08-02T09:41:19Z

@eunjae-lee my hope is that it would simply the build process, so it will be easier to make modifications and there would be less edge-cases where things don't simply work.

For example, now that I work on #241, I'd like to run in development mode while I am working on modifications. But when I run

yarn workspace @storyblok/field-plugin dev

It does not automatically compile the react and vue packages -- only @storyblok/field-plugin.

Neither does this work:

yarn build

Because the current build script does not run the copy-helpers.sh script. Of courser, this can easily be fixed by appending && ./scripts/copy-helpers.sh to the build script. But when I do so, it does not allow me to build with type errors in my project. While developing experimentally, it would be convenient to be able to compile with type errors. (I want to try out the react package first, however the vue2 and vue3 packages fails).

Of course, there are ways to work around all of these issues one-by-one (especially now when our memories are fresh), but maybe in a few years, it will be hard to make changes if the complexity of the project is too great.

Though I don't know what the best approach is here -- I haven't developed a library with multiple exports before. Maybe if we try to use type declaration files and JSdocs, we'll encounter other unexpected issues... Though maybe it's worth a try?

eunjae-lee · 2023-08-04T10:42:24Z

```shell
yarn workspace @storyblok/field-plugin dev
It does not automatically compile the react and vue packages -- only @storyblok/field-plugin.

At the root package.json, we have this:

"postinstall": "yarn build:lib",

So anyone running npm install or yarn install will get the submodules built automatically. So this is just a one time thing you encountered (just like when you switch branches and something doesn't work and it's because node_modules is out of sync)

And in the package.json of @storyblok/field-plugin, we already have a script to build helpers right after building the library.

    "build": "rm -rf dist && vite build && yarn build:helpers"

there are ways to work around all of these issues one-by-one

I don't see this as issues that we're working around, but it's that we're introducing a new thing here and encountering initial bugs or misconfigurations.

we try to use type declaration files and JSdocs

But the react helper simply cannot export .jsx file, and the vue helper cannot export .vue file. They have to be transpiled into .js files. So we still need build process, and I don't genuinely understand which difference you're suggesting here. Are you saying

build process + typescript

vs.

build process + no typescript and manually written type definitions

and suggesting the latter?

eunjae-lee · 2023-08-04T10:42:34Z

@johannes-lindgren

feat(lib): export useFieldPlugin react hook

bc9097e

vercel bot deployed to Preview July 5, 2023 16:01 View deployment

remove unnecessary .gitignore

5451066

eunjae-lee commented Jul 6, 2023

View reviewed changes

eunjae-lee requested a review from BibiSebi July 6, 2023 13:48

eunjae-lee marked this pull request as ready for review July 6, 2023 13:48

vercel bot deployed to Preview July 6, 2023 14:00 View deployment

BibiSebi approved these changes Jul 10, 2023

View reviewed changes

Merge branch 'main' into EXT-1804-export-framework-specific-helpers

e4de349

eunjae-lee merged commit c375a5c into main Jul 10, 2023

eunjae-lee deleted the EXT-1804-export-framework-specific-helpers branch July 10, 2023 12:41

vercel bot deployed to Preview July 10, 2023 12:42 View deployment

This was referenced Jul 13, 2023

chore(common): clean up scripts to build and copy helpers #227

Merged

feat(helper): add vue 2 submodule #231

Merged

johannes-lindgren reviewed Jul 25, 2023

View reviewed changes

eunjae-lee mentioned this pull request Jul 27, 2023

chore(lib): rename react helper files #237

Merged

eunjae-lee mentioned this pull request Aug 1, 2023

chore(cli): change helper folder structure #240

Merged

eunjae-lee mentioned this pull request Aug 4, 2023

chore(cli): extract vite config as a submodule to the library #244

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(lib): export useFieldPlugin react hook #224

feat(lib): export useFieldPlugin react hook #224

eunjae-lee commented Jul 5, 2023 •

edited

Loading

vercel bot commented Jul 5, 2023 •

edited

Loading

eunjae-lee Jul 6, 2023

eunjae-lee Jul 6, 2023

johannes-lindgren Jul 25, 2023

eunjae-lee Jul 6, 2023

BibiSebi Jul 10, 2023

eunjae-lee Jul 10, 2023

BibiSebi commented Jul 10, 2023

eunjae-lee commented Jul 10, 2023

BibiSebi left a comment

BibiSebi Jul 10, 2023

johannes-lindgren left a comment

johannes-lindgren Jul 25, 2023

eunjae-lee Jul 25, 2023

johannes-lindgren Jul 25, 2023

eunjae-lee commented Jul 25, 2023

eunjae-lee commented Jul 25, 2023

johannes-lindgren commented Jul 26, 2023 •

edited

Loading

eunjae-lee commented Jul 26, 2023

johannes-lindgren commented Aug 1, 2023

eunjae-lee commented Aug 1, 2023

johannes-lindgren commented Aug 2, 2023

eunjae-lee commented Aug 4, 2023

eunjae-lee commented Aug 4, 2023

feat(lib): export useFieldPlugin react hook #224

feat(lib): export useFieldPlugin react hook #224

Conversation

eunjae-lee commented Jul 5, 2023 • edited Loading

What?

Why?

Implementation Details

Approach 1

Approach 2

Approach 3

Conclusion

How I Added the React Helper

vercel bot commented Jul 5, 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

BibiSebi commented Jul 10, 2023

eunjae-lee commented Jul 10, 2023

BibiSebi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johannes-lindgren 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

eunjae-lee commented Jul 25, 2023

eunjae-lee commented Jul 25, 2023

johannes-lindgren commented Jul 26, 2023 • edited Loading

eunjae-lee commented Jul 26, 2023

johannes-lindgren commented Aug 1, 2023

eunjae-lee commented Aug 1, 2023

johannes-lindgren commented Aug 2, 2023

eunjae-lee commented Aug 4, 2023

eunjae-lee commented Aug 4, 2023

eunjae-lee commented Jul 5, 2023 •

edited

Loading

vercel bot commented Jul 5, 2023 •

edited

Loading

johannes-lindgren commented Jul 26, 2023 •

edited

Loading