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

Improvements on simpledev, live-server, etc #1543

Closed

Conversation

maxmarkus
Copy link
Contributor

@maxmarkus maxmarkus commented Aug 10, 2020

Improvements after #1449, followup of #1565

@maxmarkus maxmarkus added security/high Related to CVSSv3 security rating https://www.first.org/cvss/calculator/3.0 security always set in addition to specific security severity label, since github filtering is lacking OR labels Aug 10, 2020
@maxmarkus maxmarkus added this to the Sprint 12 milestone Aug 10, 2020
@ndricimrr ndricimrr self-assigned this Aug 10, 2020
…rity-policy-refactor

# Conflicts:
#	core/package-lock.json
#	core/webpack.config.js
@JohannesDoberer JohannesDoberer self-assigned this Aug 11, 2020
@ndricimrr
Copy link
Contributor

Should we get rid of babel.config.js or part of it 🤔 ?

@ndricimrr
Copy link
Contributor

ndricimrr commented Aug 11, 2020

Some notes I pulled while looking at this in general. Probably helpful to keep track of why we changed it for future reference.

Removed Parts:

  1. babel

  2. core-js

  3. regenerator-runtime

  4. terser

  5. live-server

History of removed parts:

  1. babel , core-js and regenerator-runtime were first added in the same PR Svelte 3 migration as follows:

    • babel is added link:

    • regenerator-runtime is added link:

    • core-js is added link :

  2. Terser is introduced later in the Improve js bundle size PR here

  3. In same PR some slight modifications to babel by adding source-map option as inline

  4. live-server : replaced with webpack-dev-server reason is here

Some results so far

There seem to be changes in bundle sizes:

  • JS bundle size before:
    image

  • JS bundle size after:
    image

Before and after images from bundle analysis:
Screenshot 2020-08-11 at 13 56 12
Screenshot 2020-08-11 at 13 55 54

Diff in size ≈ -100 KB

Also, due to removing babel build configs, luigi.js.map file which is ≈ 980 KB is also omitted from the resulting build. As it's needed for source mapping (i.e: better debugging) might consider adding source mapping through webpack's devtool as an alternative. Could help to consider this article to argue with.

On a side note need to discuss more about unsafe-inline

Copy link
Contributor

@hardl hardl left a comment

Choose a reason for hiding this comment

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

Please revert all changes that are not related to the fix of the corresponding ticket, e.g. live-server, prettier, etc.

@CLAassistant
Copy link

CLAassistant commented Aug 12, 2020

CLA assistant check
All committers have signed the CLA.

@maxmarkus maxmarkus changed the title Remove core-js + runtime, remove babel bundling Improvements on simpledev, live-server, etc Aug 14, 2020
@maxmarkus maxmarkus removed security always set in addition to specific security severity label, since github filtering is lacking OR security/high Related to CVSSv3 security rating https://www.first.org/cvss/calculator/3.0 labels Aug 14, 2020
@maxmarkus
Copy link
Contributor Author

I am creating a new PR, this one stays and will be merged afterwards with followup improvements.

@maxmarkus
Copy link
Contributor Author

This implementation has some side effects and it was for the purpose of allowing CSP in dev mode.

@maxmarkus maxmarkus closed this Aug 31, 2020
@maxmarkus maxmarkus deleted the 1449-content-security-policy-refactor branch August 31, 2020 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants