storyblok / field-plugin

Create and deploy Storyblok Field Plugin
https://www.storyblok.com/docs/plugins/field-plugins/introduction
25 stars 3 forks source link

chore(common): resolve source instead of bundle on development #264

Closed eunjae-lee closed 1 year ago

eunjae-lee commented 1 year ago

What?

This PR updates vite configs to resolve source files instead of bundles when NODE_ENV is not production (probably either development or test).

It means we don't have to run build command every time we update something. More details in the comments section.

For example, vite config of Container looks like this:

  resolve: {
    alias:
      process.env.NODE_ENV === 'production'
        ? []
        : [
          {
            find: /^@storyblok\/field-plugin$/,
            replacement: path.resolve(
              __dirname,
              '../field-plugin/src/index.ts',
            ),
          },
        ],
  },

It means that the source of field plugin library will be resolved instead of @storyblok/field-plugin inside the Container package. However, we cannot add such config to the templates, because we don't want this config inside users' created repositories, right? So I had to create separate vite configs for the templates. Couldn't find out an alternative to this approach.

Why?

JIRA: EXT-1751

How to test? (optional)

https://github.com/storyblok/field-plugin/assets/499898/4739b5cb-a3ac-4f81-bc98-1a614d23b88a

vercel[bot] commented 1 year ago

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 4, 2023 8:26am
BibiSebi commented 1 year ago

I am wondering if the extended vite config from helper functions will actually have any effect once let's say a user creates a new template and runs yarn dev.

Quite some of the code seems to be redundant, do you think @eunjae-lee it makes sense to make the alias a reusable constant?

I am also playing with the idea if it was possible to create a more dynamic/simpler way of creating these dev configs, maybe by creating a special script. But the idea is not well-thought trough. I mean something like yarn dev-internal:react and the script would generate the correct temporary configs.

I am thinking about an easier way because every time we create a new template we need to add the new extended configuration, which might be a bit cumbersome and we will also need to add this as a new step for our contributors.

eunjae-lee commented 1 year ago

I am wondering if the extended vite config from helper functions will actually have any effect once let's say a user creates a new template and runs yarn dev.

If I get your question correctly, helper is fine. When you think about the build process, we build helpers, and copy the bundled files into packages/field-plugin/dist/... At that point, helpers' vite config doesn't matter. Let me know if it's still unclear!

I am also playing with the idea if it was possible to create a more dynamic/simpler way of creating these dev configs, maybe by creating a special script. But the idea is not well-thought trough. I mean something like yarn dev-internal:react and the script would generate the correct temporary configs.

I like the idea of a script. I didn't want to overcomplicate this pull-request from the beginning. Now that you see what's going on here, and you agree on having a better way to have this config for the sake of better maintenance, I'm happy to improve this pull-request. Ping me if you have some ideas, while I work on it :)

eunjae-lee commented 1 year ago

cc @BibiSebi (forgot to ping you)

demetriusfeijoo commented 1 year ago

@eunjae-lee, everything seemed nice to me.

The script file was a nice idea, I tested it locally, and worked pretty well.

I left one note for the sake of understanding better the SDK, but nothing which would break anything as well.

BibiSebi commented 1 year ago

I am wondering if the extended vite config from helper functions will actually have any effect once let's say a user creates a new template and runs yarn dev.

If I get your question correctly, helper is fine. When you think about the build process, we build helpers, and copy the bundled files into packages/field-plugin/dist/... At that point, helpers' vite config doesn't matter. Let me know if it's still unclear!

I am also playing with the idea if it was possible to create a more dynamic/simpler way of creating these dev configs, maybe by creating a special script. But the idea is not well-thought trough. I mean something like yarn dev-internal:react and the script would generate the correct temporary configs.

I like the idea of a script. I didn't want to overcomplicate this pull-request from the beginning. Now that you see what's going on here, and you agree on having a better way to have this config for the sake of better maintenance, I'm happy to improve this pull-request. Ping me if you have some ideas, while I work on it :)

What about merging this and creating another ticket to improve this? It does not have to be now, but in case we have the time and motivation we can look into it in the next sprint. As long as it works, I think we can merge it like this, but, we need to include the information inside about the steps inside the README for contributors. What do you think. @eunjae-lee.

BibiSebi commented 1 year ago

Never mind, I see you already started working on it :)