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.

Sign up for GitHub

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

CLI: Improve init for svelte #14161

Merged
merged 2 commits into from
Mar 16, 2021
Merged

CLI: Improve init for svelte #14161

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ea8e9d6
Improve sb init for svelte
j3rem1e Mar 7, 2021
99d8293
Add extra options to baseGenerator
tooppaaa Mar 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 39 additions & 1 deletion lib/cli/src/generators/SVELTE/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,47 @@
import fse from 'fs-extra';
import { logger } from '@storybook/node-logger';

import { baseGenerator, Generator } from '../baseGenerator';

const generator: Generator = async (packageManager, npmOptions, options) => {
baseGenerator(packageManager, npmOptions, options, 'svelte', {
await baseGenerator(packageManager, npmOptions, options, 'svelte', {
extraPackages: ['svelte', 'svelte-loader'],
extraAddons: ['@storybook/addon-svelte-csf'],
});

let conf = fse.readFileSync('./.storybook/main.js').toString();

// add *.stories.svelte
conf = conf.replace(/js\|jsx/g, 'js|jsx|svelte');
Copy link
Contributor

Choose a reason for hiding this comment

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

This means we can break Svelte CLI while touching lib\cli\src\generators\configure.ts Do you think it's best to pass an option "extension" that defaults to js, jsx, ts, txs ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My first implementation was checking if "framework is svelte" (there is already a special case for angular in this file), my 2nd implementation was doing that (a special properties for extensions), and my third is this PR :)


let requirePreprocessor;
let preprocessStatement = 'undefined';

// svelte.config.js ?
if (fse.existsSync('./svelte.config.js')) {
logger.info("Configuring preprocessor from 'svelte.config.js'");

requirePreprocessor = `const preprocess = require("../svelte.config.js").preprocess;`;
preprocessStatement = 'preprocess';
} else {
// svelte-preprocess dependencies ?
const packageJson = packageManager.retrievePackageJson();
if (packageJson.devDependencies && packageJson.devDependencies['svelte-preprocess']) {
logger.info("Configuring preprocessor with 'svelte-preprocess'");

requirePreprocessor = 'const sveltePreprocess = require("svelte-preprocess");';
preprocessStatement = 'sveltePreprocess()';
}
}

const svelteOptions = ` "svelteOptions": {\n preprocess: ${preprocessStatement},\n },`;

if (requirePreprocessor) {
conf = `${requirePreprocessor}\n\n${conf}`;
}

conf = conf.replace(/\],/, `],\n${svelteOptions}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is also not very robust. Same as the in the configure method of lib\cli\src\generators\baseGenerator.ts you have the capacity to add "extraMain" args to your main. With a small refactor, you won't need to know about ./.storybook/main.js in this file

Copy link
Member

Choose a reason for hiding this comment

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

That's why you're the MVP @tooppaaa ❤️

Thanks so much for the thoughtful review!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, the main.js is generating from static properties (with JSON.stringify).

However, in my case, I want to inject :

  • An import statement / require
  • A Function (which can't be generated by JSON.stringify), so I have to format a string and format it correctly
const sveltePreprocess = require('svelte-preprocess');

module.exports = {
   ...
   svelteOptions: {
     preprocessor: sveltePreprocess()
  },
  ..
}

That's why, for me at least, it wasn't a "small refactor" but I have to rewrite most of the logic inside configure.js 😅
I didn't found jest test around this function, so this refactoring could be risky

I can work on that, but how do you see the implementation of configure for my case ?

fse.writeFileSync('./.storybook/main.js', conf);
};

export default generator;