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

feat: use cache file to replace Symfony commands #47

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

jon-ht
Copy link

@jon-ht jon-ht commented Feb 8, 2025

Hi ! First thanks for this project, it looks very promising.

I'd like to use it in my projects, but I encountered some limitations.

When running npm run storybook / yarn storybook, it is expected to have a local PHP binary to run Symfony commands. I don't have PHP on my machine as our projects use Docker Compose.

Internal command can accept options

const prepareSymfonyCommand = (command: string, inputs: string[] = [], options: CommandOptions = {}) => {
const finalOptions = {
...defaultOptions,
...options,
};
return [finalOptions.php, finalOptions.script, command]
.concat([...inputs, '-v'])
.map((part) => `'${part}'`)
.join(' ');
};

But there is no way to provide custom PHP path nor use docker exec commands.


So here I am, with a proposal to not rely on PHP binary, and only use Symfony as we do every day : with cache files.

This PR aims to generate and use a file containing all configurations required by JS scripts. The file structure would be as follow

/path/to/app/var/cache/dev/storybook/symfony_parameters.json
{
  "kernel_project_dir": "\/path\/to\/app",
  "storybook_config": {
    "sandbox": {
      "allowedFunctions": [
        "dump"
      ],
      "allowedTags": [],
      "allowedFilters": [],
      "allowedProperties": [],
      "allowedMethods": []
    },
    "cache": "\/path\/to\/app\/var\/cache\/dev\/storybook\/twig"
  },
  "twig_config": {
    "file_name_pattern": [
      "*.twig"
    ],
    "default_path": "\/path\/to\/app\/templates",
    "globals": {
      "some_global": {
        "value": "foo"
      }
    },
    "form_themes": [
      "form_div_layout.html.twig"
    ],
    "autoescape_service": null,
    "autoescape_service_method": null,
    "cache": "\/path\/to\/app\/var\/cache\/dev\/twig",
    "charset": "UTF-8",
    "debug": true,
    "strict_variables": true,
    "date": {
      "format": "F j, Y H:i",
      "interval_format": "%d days",
      "timezone": null
    },
    "number_format": {
      "decimals": 0,
      "decimal_point": ".",
      "thousands_separator": ","
    }
  },
  "twig_component_config": {
    "defaults": {
      "Front\\Twig\\Components\\": {
        "template_directory": "components\/",
        "name_prefix": "Shared"
      }
    },
    "anonymous_template_directory": "components\/",
    "profiler": true,
    "controllers_json": null
  }
}

Every command providing a configuration in symfony.ts is now a distinct entry in this symfony_parameters.json (any better name would be accepted)


WDYT about this ? This is still a draft, and more code should be adjusted in order to remove symfony.ts file and all related processes


TODO

  • Remove remaining types/functions from symfony.ts based on command execution
  • Update symfony.test.ts
  • Remove GeneratePreviewCommand
  • Update docs
  • Update recipe for .storybook/main.ts

@jon-ht jon-ht force-pushed the feat-use-cache-file-to-replace-commands branch from db97903 to 4c5fb18 Compare February 8, 2025 02:46
Comment on lines +28 to +50
private function getConfigForExtension(ExtensionInterface $extension, ContainerBuilder $container)
{
$extensionAlias = $extension->getAlias();

if (isset($this->extensionConfig[$extensionAlias])) {
return $this->extensionConfig[$extensionAlias];
}

$configs = $container->getExtensionConfig($extensionAlias);

$configuration = $extension instanceof ConfigurationInterface ? $extension : $extension->getConfiguration($configs, $container);

return $this->extensionConfig[$extensionAlias] = (new Processor())->processConfiguration($configuration, $configs);
}

private function getConfig(ContainerBuilder $container, ExtensionInterface $extension): array
{
return $container->resolveEnvPlaceholders(
$container->getParameterBag()->resolveValue(
$this->getConfigForExtension($extension, $container)
), true
);
}
Copy link
Author

@jon-ht jon-ht Feb 8, 2025

Choose a reason for hiding this comment

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

This process is based on debug:config command.

I'm not really sure about the implementation. It gives me expected output, but there could be other ways to achieve this. Let me know if it could be improved

Comment on lines +18 to +25
public function __invoke(): Response
{
$this->eventDispatcher->dispatch(new GeneratePreviewEvent());

$content = $this->twig->render('@Storybook/preview.html.twig');

return new Response($content);
}
Copy link
Author

@jon-ht jon-ht Feb 15, 2025

Choose a reason for hiding this comment

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

Same process as storybook:generate-preview. This command should be removed before merging

Comment on lines +7 to +8
projectDir?: string;
storybookCachePath?: string;
Copy link
Author

Choose a reason for hiding this comment

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

projectDir is now provided by main.js|ts file

storybookCachePath is the path var/cache folder of StorybookBundle. I need this because my project uses Single Kernel approach

Here is how I configured framework.options values

options: {
    // 👇 Here configure the framework
    symfony: {
        projectDir: path.resolve(__dirname, '..'),
        storybookCachePath: path.resolve(__dirname, '../var/cache/etu/dev/storybook'),
        server: 'https://www.myapp.lh:8005',
        proxyPaths: [
            '/assets',
        ],
        additionalWatchPaths: [
            'assets',
        ]
    }
},

Comment on lines 91 to 98
const fetchUrl = new URL(`${server}/_storybook/preview`);

type StorybookBundleConfig = {
storybook: {
runtime_dir: string;
};
const response = await fetch(fetchUrl, {
method: 'GET',
headers: {
Accept: 'text/html',
},
});
Copy link
Author

Choose a reason for hiding this comment

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

I'm having a hard time with this. My project runs in HTTPS with self signed certificate

Despite secure: false option in createProxyMiddleware, I get this error when running Storybook

front$ yarn storybook
yarn run v1.22.22
$ sb dev -p 6006 --no-open --disable-telemetry
@storybook/core v8.5.3

(node:33983) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
info => Starting manager..
WARN No story files found for the specified pattern: stories/**/*.mdx
info => Starting preview..
[HPM] Proxy created: /  -> https://www.myapp.lh:8005
[HPM] Subscribed to http-proxy events: [ 'error', 'close' ]
[HPM] Proxy created: /  -> https://www.myapp.lh:8005
[HPM] Subscribed to http-proxy events: [ 'error', 'close' ]
[HPM] Proxy created: /  -> https://www.myapp.lh:8005
[HPM] Subscribed to http-proxy events: [ 'error', 'close' ]
info Addon-docs: using MDX3
info => Using implicit CSS loaders
info => Using default Webpack5 setup
<i> [webpack-dev-middleware] wait until bundle finished
<e> [webpack-dev-middleware] TypeError: fetch failed
<e>     at node:internal/deps/undici/undici:13392:13
<e>     at process.processTicksAndRejections (node:internal/process/task_queues:105:5)
<e>     at async base (/space/myapp/front/node_modules/@sensiolabs/storybook-symfony-webpack5/src/server/framework-preset.ts:93:11)
<e>     at async base (/space/myapp/front/node_modules/@sensiolabs/storybook-symfony-webpack5/src/server/framework-preset.ts:93:11) {
<e>   [cause]: Error: self-signed certificate
<e>       at TLSSocket.onConnectSecure (node:_tls_wrap:1679:34)
<e>       at TLSSocket.emit (node:events:518:28)
<e>       at TLSSocket._finishInit (node:_tls_wrap:1078:8)
<e>       at ssl.onhandshakedone (node:_tls_wrap:864:12) {
<e>     code: 'DEPTH_ZERO_SELF_SIGNED_CERT'
<e>   }
<e> }

This would be the last piece for moving from PHP binary to cached files

Copy link
Author

Choose a reason for hiding this comment

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

Resolved by providing NODE_TLS_REJECT_UNAUTHORIZED=0 to storybook script

{
    "scripts": {
        "storybook": "NODE_TLS_REJECT_UNAUTHORIZED=0 sb dev -p 6006 --no-open --disable-telemetry"
    }
}

@jon-ht jon-ht force-pushed the feat-use-cache-file-to-replace-commands branch from af6549d to 1c9fe90 Compare February 15, 2025 10:34
Comment on lines 45 to 51
/**
* When Storybook and Symfony are not on the same host (e.g. Symfony is Dockerized), mounted paths may differ.
* This option allows to map paths from Symfony host to Storybook host.
*/
templatePathAliases?: {
[p: string]: string;
};
Copy link
Author

Choose a reason for hiding this comment

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

As my Symfony app is dockerized, my host path differs from mounted path

  • On machine: /path/to/project/front
  • On Docker images: /application/front

This allows me to have this

templatePathAliases: {
    [path.resolve(__dirname, '..')]: '/application/front',
}

Comment on lines +28 to +34
const twigPaths: string[] = Object.keys(symfonyConfig.twig_config.paths).map((key) => {
if (key.startsWith(symfonyOptions.projectDir)) {
return `${key}/`;
}

return `${symfonyOptions.projectDir}/${key}/`;
});
Copy link
Author

Choose a reason for hiding this comment

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

I don't know if I misconfigured my Symfony app, but values from symfonyConfig.twig_config.paths already contains symfonyOptions.projectDir

Without this change, path is duplicated

Here is my twig.yaml

twig:
    file_name_pattern: '*.twig'
    default_path: '%kernel.project_dir%/templates'
    paths:
        '%kernel.project_dir%/templates': Shared

Comment on lines +18 to +27
const resolvePathAlias = (file: string) => {
for (const [alias, resolvedPath] of Object.entries(this.templatePathAliases)) {
if (file.startsWith(alias)) {
return file.replace(alias, resolvedPath);
}
}
return file;
}

const resolvedFile = resolvePathAlias(file);
Copy link
Author

Choose a reason for hiding this comment

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

Replace host path with Docker path

This must be adapted for additional watch paths

Ignoring additional watch path '/application/front/assets': path doesn't exists.

);

const runtimeDir = (await getBundleConfig()).runtime_dir;
Copy link
Author

Choose a reason for hiding this comment

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

Not sure about the usage of runtime_dir. I didn't find anything in source code

@jon-ht jon-ht force-pushed the feat-use-cache-file-to-replace-commands branch from 9ae6e11 to dc17a81 Compare February 25, 2025 21:35
@jon-ht jon-ht marked this pull request as ready for review February 25, 2025 21:51
@jon-ht jon-ht force-pushed the feat-use-cache-file-to-replace-commands branch 2 times, most recently from 26e9f49 to c31f352 Compare February 25, 2025 22:27
@jon-ht jon-ht force-pushed the feat-use-cache-file-to-replace-commands branch from c31f352 to 78eb6c2 Compare February 26, 2025 08:05
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.

1 participant