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

web/library: Remove worker-loader #1069

Merged
merged 1 commit into from Sep 6, 2018
Merged

Conversation

@edmorley
Copy link
Member

edmorley commented Sep 2, 2018

Since the top-level presets aim to cover the 80% case, and web workers are still less common. If after Neutrino 9's release there ends up being demand for them, we'd be better off having a separate opt-in preset (similar to @neutrinojs/pwa).

@edmorley edmorley added this to the v9 milestone Sep 2, 2018
@edmorley edmorley self-assigned this Sep 2, 2018
@edmorley edmorley requested a review from eliperelman Sep 2, 2018
@eliperelman

This comment has been minimized.

Copy link
Member

eliperelman commented Sep 4, 2018

Could you elaborate on the benefit to removing this? Do you feel like our assumptions here make it difficult to override this in the future?

@edmorley

This comment has been minimized.

Copy link
Member Author

edmorley commented Sep 4, 2018

Ah good question. The reason was just that I don't think many users will be using it, and that it's another dependency that has to be downloaded and then require()ed during the build. It also cumulatively adds to the overall cognitive overhead of understanding the web preset and the resultant --inspect output.

ie: This was just a continuation of my ongoing work to slim down Neutrino 9 as much of possible compared to the accumulation of features/complexity that we'd ended up with Neutrino 8 (whose neutrino build --inspect-new webpack config for the React preset is 400 lines long, compared to 240 lines for the equivalent on master).

If you felt strongly about leaving it then, then happy to do so. Do you know of many projects that use web workers with Neutrino? :-)

@eliperelman

This comment has been minimized.

Copy link
Member

eliperelman commented Sep 4, 2018

@edmorley I have in the past, which is why I had added it, since it gave a way to use workers with zero config. That said, I haven't used it in a while, and don't have a major issue removing it. My biggest concern was being able to import as worker with no config. 😄

@edmorley

This comment has been minimized.

Copy link
Member Author

edmorley commented Sep 5, 2018

I think in the future a combined opt-in preset for various worker types might be nice to have? (And consolidate with @neutrinojs/pwa?)

Since the top-level presets aim to cover the 80% case, and web workers
are still less common. If after Neutrino 9's release there ends up
being demand for them, we'd be better off having a separate opt-in
preset (similar to `@neutrinojs/pwa`).
@edmorley edmorley force-pushed the edmorley:rm-worker-loader branch from 68cbbc9 to 41fe707 Sep 6, 2018
@edmorley edmorley merged commit 3b323e2 into neutrinojs:master Sep 6, 2018
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details
@edmorley edmorley deleted the edmorley:rm-worker-loader branch Sep 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.