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

Avoid eval #1141

Merged
merged 2 commits into from
Nov 24, 2018
Merged

Avoid eval #1141

merged 2 commits into from
Nov 24, 2018

Conversation

nicksellen
Copy link
Member

What does this PR do?

CSP is upset about eval, and apparantly if we make this change it'll be happy again :)

See webpack/webpack#6461 (comment)

Links to related issues

#1068

Checklist

  • added a test, or explain why one is not needed/possible...
  • no unrelated changes
  • asked someone for a code review
  • joined #karrot-dev channel at https://slackin.yunity.org
  • added an entry to CHANGELOG.md (description, pull request link, username(s))

Otherwise this generates some js that uses eval and upsets csp
@codecov
Copy link

codecov bot commented Nov 23, 2018

Codecov Report

Merging #1141 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1141   +/-   ##
=======================================
  Coverage   60.87%   60.87%           
=======================================
  Files         224      224           
  Lines        3737     3737           
  Branches      633      633           
=======================================
  Hits         2275     2275           
  Misses       1461     1461           
  Partials        1        1

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0c17e42...c00e33a. Read the comment docs.

@@ -70,6 +70,9 @@ module.exports = {
chunkFilename: 'assets/js/[id].[chunkhash].js',
pathinfo: false,
},
node: {
Copy link
Member

Choose a reason for hiding this comment

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

We could also try node: false and see if all dependencies cope with it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems ok so far! It builds, and clicking around locally is ok. I'll push the change.

@tiltec tiltec merged commit f382689 into master Nov 24, 2018
@tiltec tiltec deleted the avoid-eval branch November 24, 2018 10:26
@nicksellen
Copy link
Member Author

Nope, has an error. On https://deploy-preview-1141--karrot.netlify.com/ when trying to edit my profile.

ReferenceError: "process is not defined"

vuelidate tries to access process.env.BUILD in ./vuelidate/src/withParamsBrowser.js

Seems they should fix it, see vuelidate/vuelidate#365, but we can force it. I guess "web" is the correct value.

nicksellen added a commit that referenced this pull request Nov 24, 2018
@nicksellen nicksellen restored the avoid-eval branch November 24, 2018 10:31
@nicksellen nicksellen deleted the avoid-eval branch November 24, 2018 10:36
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.

2 participants