Skip to content
This repository has been archived by the owner on Oct 9, 2020. It is now read-only.

plugin-css requires a fresh builder instance each time #102

Open
arackaf opened this issue Aug 19, 2016 · 9 comments
Open

plugin-css requires a fresh builder instance each time #102

arackaf opened this issue Aug 19, 2016 · 9 comments

Comments

@arackaf
Copy link

arackaf commented Aug 19, 2016

Hi Guy. I'm seeing what I think is a bug. Essentially, the plugin-css requires a fresh builder instance each time. If you create bundles with css dependencies on the same builder instance, some lines seem to get crossed, and css ends up in incorrect places.

Specifically, consider

for (let name of namesToOptimize) {
    let builder = new Builder('../' + baseDirectory);
    builder.config(configToUse);

with calls to

await builder
          .bundle(`${name}`, outputPath, configToUse)
          .then(result => {

below, inside the loop body. If I move

let builder = new Builder('../' + baseDirectory);
builder.config(configToUse);

above the for loop, the problems described above surface.

Is this expected behavior? Do you need me to repro this somewhere in GitHub?

@guybedford
Copy link
Member

There may well be bugs with running multiple builds at the same time as the individual plugin instance is designed to work against a single build.

We may need some way to key the build instance itself in the internal plugin cache layer here.

@arackaf
Copy link
Author

arackaf commented Aug 21, 2016

Simpler solution might be to just error out if the plugin is run more than once on the same builder instance.

@guybedford
Copy link
Member

The issue shouldn't actually be the instance, but rather having multiple builds running at the same time?

Or even with waiting for the build to complete are you getting issues using the same builder instance?

@arackaf
Copy link
Author

arackaf commented Aug 22, 2016

I just checked - every call to bundle is indeed preceded by await so I think there should never have been more than one running at the same time.

@guybedford
Copy link
Member

I've done some further testing and can confirm this is entirely to do with parallel builds not being supported.

I think I have some ideas going forward, but it will require some changes to the builder API so don't want to rush this.

@arackaf
Copy link
Author

arackaf commented Aug 30, 2016

This tool is pretty outstanding as is. Preventing parallel builds is easy with the simplest of helper methods - the best work to reward ratio may be to just error out on parallel build attempt. My $0.02 :)

@guybedford
Copy link
Member

Thanks, unfortunately the detection problem is just as hard though!

@guybedford
Copy link
Member

I've included a fix for this in 6be8e64, although there may still be some race conditions remaining. Let me know if it helps though!

@arackaf
Copy link
Author

arackaf commented Sep 23, 2016

Sure - thanks much!

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

No branches or pull requests

2 participants