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

Tree-shaken CSS causes build breaks #441

Closed
tivac opened this issue Jun 29, 2018 · 3 comments

Comments

Projects
None yet
1 participant
@tivac
Copy link
Owner

commented Jun 29, 2018

If you import CSS in a module that is later completely tree-shaken away it'll cause an error when rollup tries to get the dependencies for the non-existent file.

const used = processor.dependencies(start).concat(start);

is where we run into trouble.

This fixes it by removing any unknown-to-the-processor files from the array before trying to get any dependencies.

css.filter((start) => processor.files[start]).forEach(/*...*/)

@tivac tivac self-assigned this Jun 29, 2018

tivac added a commit that referenced this issue Jul 2, 2018

test: try to replicate #441
No luck though, need to get a better repro.
@tivac

This comment has been minimized.

Copy link
Owner Author

commented Jul 3, 2018

Looks like it actually has to do with (surprise surprise) files being removed from the processor due to having multiple other files depend on them. May need to go back to the runs-based method of clearing out files for reprocessing.

@tivac

This comment has been minimized.

Copy link
Owner Author

commented Jul 3, 2018

processor.dependencies(id)
.concat(id)
.forEach((file) => processor.remove(file));

needs to be

processor.remove(id);

There's also a weird timing issue around svelte preprocess where it seems like the async/await isn't being waited on properly?

@tivac

This comment has been minimized.

Copy link
Owner Author

commented Jul 5, 2018

I changed the code so it'd work correctly when a file's dependencies change in watch mode, but that broke the handling of a passed Processor instance.

Nothing to do w/ tree-shaking, blerp.

@tivac tivac closed this in #442 Jul 5, 2018

tivac added a commit that referenced this issue Jul 5, 2018

fix: Improve rollup dependency removal (#442)
* test: try to replicate #441

No luck though, need to get a better repro.

* Got a failing test!

* chore: 🔒 file wut

* fix: Only remove the file and its dependents

Not sure why I changed it to the file and all its dependencies instead of just the default "file and its dependants" behavior, but that was the WRONG choice.

* fix: handle both removal cases

See comment on line 53 for the reasoning, it's goofy.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.