Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

Fixing workflow #94 (under review) #186

Closed
wants to merge 3 commits into from
Closed

Conversation

unlight
Copy link

@unlight unlight commented Feb 15, 2017

This pull request trying to fix bugs described in #94

  • CssPlugin emit HMR event for non-modified files (see screenshot bug1)
  • HMR does not work properly if css was converted to raw content (see screenshot bug2)
  • decide: __fsbx_css serialize or global FuseBox.includeStyle
  • HMR event for RawPlugin
  • Unit tests

Given

App component

@Component({
    selector: 'app',
    template: require('./app.component.html'),
    styles: [require('./app.component.scss')],
})
export class AppComponent {
    name = 'Angular';
}

Config (full file)

const fuseBox = _.memoize(function createFuseBox(options = {}) {
    const config: any = _.get(options, 'config');
    const entry = _.get(options, 'entry', 'app');
    const plugins: any[] = [
        [
            /\.ts$/,
            GulpPlugin([
                (file) => g.preprocess({ context: config }),
            ]),
        ],
        [
            /\.component\.scss$/,
            SassPlugin({ sourceMap: false }),
            PostCSS(postcssPlugins()),
            GulpPlugin([
                (file) => g.if(!config.devMode, g.csso()),
            ]),
            customRawPlugin(),
        ],
        [
            /\.scss$/,
            SassPlugin({}),
            ...cssChain(),
        ],
        [
            /\.css$/,
            ...cssChain(),
        ],
        HTMLPlugin({ useDefault: false }),
    ];
    const settings = {
        homeDir: 'src',
        log: false,
        sourceMap: {
            bundleReference: `${entry}.js.map`,
            outFile: `./${config.dest}/${entry}.js.map`,
        },
        tsConfig: `${__dirname}/tsconfig.json`,
        cache: true,
        outFile: `./${config.dest}/${entry}.js`,
        plugins: plugins,
    };
    if (!config.devMode) {
        if (config.minify) {
            plugins.push(UglifyJSPlugin({}));
        }
    }
    const fuse = FuseBox.init({ ...settings, ...options });
    return fuse;
});

Bug 1

CssPlugin emit HMR event for non-modified files (style.scss was not modified)

Bug 2

HMR does not work properly if css was converted to raw content

I know that something related to css and hmr is coming (bundle, hmr plugins etc.) maybe his fixes will be not necessary.
Please note, I've done a lot of hacks in config to fit my requirements. Maybe I'm doing it wrong.
Please review the changes.

@unlight unlight changed the title Fixing workflow fuse-box/fuse-box#94 (under review) Fixing workflow #94 (under review) Feb 15, 2017
@nchanged
Copy link
Contributor

Have to wait until this one is merged #158

Thanks!

@nchanged
Copy link
Contributor

Please consider the new changes! Thanks!

@unlight
Copy link
Author

unlight commented Feb 17, 2017

@nchanged
A revision is needed here 🐤
My questions in description were:

  1. Is it ok to expose __fsbx_css as FuseBox.includeStyle?
  2. How to write unit tests for client side fusebox plugins (for fusebox-hot-reload in particular)?
    Other questions are non relevant anymore, becasue CssPlugin and RawCssPlugin are used in chain, so cache must be implemented there.

# Conflicts:
#	modules/fusebox-hot-reload/index.js
#	src/modules/fsbx-default-css-plugin/index.ts
@unlight
Copy link
Author

unlight commented Feb 18, 2017

First of all, part of config.

const plugins: any[] = [
    [
        /\.component\.scss$/,
        SassPlugin({ sourceMap: false }),
        {
            transform: (file: File) => {
                file.contents = `module.exports = ${JSON.stringify(file.contents)}`;
                file.context.emitJavascriptHotReload(file);
            }
        }
    ],
    [
        /\.scss$/,
        SassPlugin({}),
        CSSPlugin({ write: prod === true })
    ],
];

Notice 2 things.

  1. *.component.scss (e.g. app.component.scss) inlines to component file
  2. other *.scss (e.g. style.scss) files will be written in prod mode, or inserted as text to head styles in dev.

The goal is to do properly HMR of *.component.scss.

Steps to reproduce incorrect behaviour, we are in dev mode {write: false}:

  1. Edit style.scss, All is OK, work as expected (style changed from S1 to S2)
  2. Edit app.component.scss
  3. HMR starting FuseBox.import(FuseBox.mainFile)
  4. Execution hits to line require("./style.scss");
  5. require executes __fsbx_css("style.scss", styleContent)
  6. styleContent is equal to S1, because content of style.scss module was not
    updated on step 1 (was executed only __fsbx_css(...),

To fix this bug we need a fix - update dynamic content in hot-reload module.

if (data.type === "css" && __fsbx_css) {
    var newContent = `__fsbx_css(${JSON.stringify(data.path)},${JSON.stringify(data.content)})`;
    FuseBox.dynamic(data.path, newContent); // will not work.
    __fsbx_css(data.path, data.content)
}

But solution above will not work, becase on step 4 somewhere inside of fusebox following will be called:

var res = new Function('__fbx__dnm__', 'exports', 'require', 'module', '__filename', '__dirname', '__root__', str);
res(true, exports, require, module, __filename, __dirname, __root__);

str is __fsbx_css("style.scss",'...' from above.
Result of call of res(...) - Uncaught ReferenceError: __fsbx_css is not defined.

(function(__fbx__dnm__,exports,require,module,__filename,__dirname,__root__
/**/) {
__fsbx_css("style.scss","...")
})

To prevent this failing need (2 options):

  1. Expose __fsbx_css as global
    // fsbx-default-css-plugin/index.js
    FuseBox.includeStyle = __fsbx_css;
    // fusebox-hot-reload/index.js 
     if (data.type === "css" && __fsbx_css) {
         var newContent = `FuseBox.includeStyle(${JSON.stringify(data.path)},${JSON.stringify(data.content)})`;
         FuseBox.dynamic(data.path, newContent);
         FuseBox.includeStyle(data.path, data.content);
     }
    
  2. Serialize __fsbx_css function then deserialize in call as iife
if (data.type === "css" && __fsbx_css) {
    var newContent = '(' + __fsbx_css.toString() + ')('+JSON.stringify(data.path)+', '+JSON.stringify(data.content)+')';
    FuseBox.dynamic(data.path, newContent);
    __fsbx_css(data.path, data.content)
}

If is it ok, then one thing left - unit tests.

Other point are non relevant to css plugin or hot reload. I'll create separate PR when ready.

@unlight
Copy link
Author

unlight commented Feb 21, 2017

@nchanged ping
I'm not sure how it will work with new CSSPlugin options.

@nchanged
Copy link
Contributor

So what does it do?

@unlight
Copy link
Author

unlight commented Feb 21, 2017

  1. Need to decide __fsbx_css serialized or global FuseBox.includeStyle
  2. How to unit tests this

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants