search

apostrophecms / sanitize-html

Clean up user-submitted HTML, preserving whitelisted elements and whitelisted attributes on a per-element basis. Built on htmlparser2 for speed and tolerance
MIT License
3.84k stars 353 forks source link

minify script no working — ERROR: Unexpected token: keyword «const» #300

Closed mjbvz closed 4 years ago

mjbvz commented 5 years ago

Repo

  1. Clone repo (2b7d02216f3f1afb2abdcbe00222a5a6848008d0)
  2. npm install
  3. npm run minify

Bug The script fails

matb@~/projects/sanitize-html$npm run minify

> sanitize-html@1.20.1 minify /Users/matb/projects/sanitize-html
> npm run build && uglifyjs dist/sanitize-html.js > dist/sanitize-html.min.js

> sanitize-html@1.20.1 build /Users/matb/projects/sanitize-html
> make clean && make all && npm run prepare && browserify dist/index.js > dist/sanitize-html.js --standalone 'sanitizeHtml'

rm -rf dist
mkdir -p dist
./node_modules/.bin/babel src -d dist
src/index.js -> dist/index.js

> sanitize-html@1.20.1 prepare /Users/matb/projects/sanitize-html
> true

Parse error at dist/sanitize-html.js:2863,0
const foreignModeIntegrationPoints = [
^
ERROR: Unexpected token: keyword «const»
    at JS_Parse_Error.get (eval at <anonymous> (/Users/matb/projects/sanitize-html/node_modules/uglify-js/tools/node.js:20:1), <anonymous>:71:23)
    at fatal (/Users/matb/projects/sanitize-html/node_modules/uglify-js/bin/uglifyjs:298:27)
    at run (/Users/matb/projects/sanitize-html/node_modules/uglify-js/bin/uglifyjs:241:9)
    at Object.<anonymous> (/Users/matb/projects/sanitize-html/node_modules/uglify-js/bin/uglifyjs:167:5)
    at Module._compile (internal/modules/cjs/loader.js:776:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:787:10)
    at Module.load (internal/modules/cjs/loader.js:653:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:593:12)
    at Function.Module._load (internal/modules/cjs/loader.js:585:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:829:12)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! sanitize-html@1.20.1 minify: `npm run build && uglifyjs dist/sanitize-html.js > dist/sanitize-html.min.js`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the sanitize-html@1.20.1 minify script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
boutell commented 5 years ago

Interesting. Offhand I don't see how that would happen since babel is configured to use the env preset, which ought to compile all ES6-isms.

On Fri, Aug 16, 2019 at 3:03 PM Matt Bierner notifications@github.com wrote:

Repo

  1. Clone repo (2b7d022 https://github.com/apostrophecms/sanitize-html/commit/2b7d02216f3f1afb2abdcbe00222a5a6848008d0 )
  2. npm install
  3. npm run minify

Bug The script fails

matb@~/projects/sanitize-html$npm run minify

sanitize-html@1.20.1 minify /Users/matb/projects/sanitize-html

npm run build && uglifyjs dist/sanitize-html.js > dist/sanitize-html.min.js

sanitize-html@1.20.1 build /Users/matb/projects/sanitize-html

make clean && make all && npm run prepare && browserify dist/index.js > dist/sanitize-html.js --standalone 'sanitizeHtml'

rm -rf dist

mkdir -p dist

./node_modules/.bin/babel src -d dist

src/index.js -> dist/index.js

sanitize-html@1.20.1 prepare /Users/matb/projects/sanitize-html

true

Parse error at dist/sanitize-html.js:2863,0

const foreignModeIntegrationPoints = [

^

ERROR: Unexpected token: keyword «const»

at JS_Parse_Error.get (eval at <anonymous> (/Users/matb/projects/sanitize-html/node_modules/uglify-js/tools/node.js:20:1), <anonymous>:71:23)

at fatal (/Users/matb/projects/sanitize-html/node_modules/uglify-js/bin/uglifyjs:298:27)

at run (/Users/matb/projects/sanitize-html/node_modules/uglify-js/bin/uglifyjs:241:9)

at Object.<anonymous> (/Users/matb/projects/sanitize-html/node_modules/uglify-js/bin/uglifyjs:167:5)

at Module._compile (internal/modules/cjs/loader.js:776:30)

at Object.Module._extensions..js (internal/modules/cjs/loader.js:787:10)

at Module.load (internal/modules/cjs/loader.js:653:32)

at tryModuleLoad (internal/modules/cjs/loader.js:593:12)

at Function.Module._load (internal/modules/cjs/loader.js:585:3)

at Function.Module.runMain (internal/modules/cjs/loader.js:829:12)

npm ERR! code ELIFECYCLE

npm ERR! errno 1

npm ERR! sanitize-html@1.20.1 minify: npm run build && uglifyjs dist/sanitize-html.js > dist/sanitize-html.min.js

npm ERR! Exit status 1

npm ERR!

npm ERR! Failed at the sanitize-html@1.20.1 minify script.

npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/sanitize-html/issues/300?email_source=notifications&email_token=AAAH27PER3OCTX244V663X3QE32XDA5CNFSM4IML7ZD2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HFW2NBQ, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAH27M345CQTXUBPMH44ZDQE32XDANCNFSM4IML7ZDQ .

--

Thomas Boutell, Chief Software Architect Pronouns: he / him / his P'unk Avenue | (215) 755-1330 | punkave.com

mjbvz commented 5 years ago

The error is for const foreignModeIntegrationPoints which is defined in dom-serializer which gets pulled in by htmlparser2. I think the specific problem is that the build does the following:

  1. Run babel (which converts to es5)
  2. Run output of step 1 through browserify (to generate a single file)
  3. Run output of step 2 uglifyjs

Step 2 adds a const back in

boutell commented 5 years ago

Sounds like steps 1 and 2 should be swapped.

On Sat, Aug 17, 2019 at 12:19 PM Matt Bierner notifications@github.com wrote:

The error is for const foreignModeIntegrationPoints which is defined in dom-serializer which gets pulled in by htmlparser2. I think the specific problem is that the build does the following:

  1. Run babel (which converts to es5)
  2. Run output or step 1 through browserify (to generate a single file)
  3. Run output of step 2 uglifyjs

Step 2 adds a const back in

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/sanitize-html/issues/300?email_source=notifications&email_token=AAAH27IY7XQPP3GGDJEJRI3QFAQHLA5CNFSM4IML7ZD2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4QOUKI#issuecomment-522250793, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAH27KF27Z4OHSUCY2X47TQFAQHLANCNFSM4IML7ZDQ .

--

Thomas Boutell, Chief Software Architect Pronouns: he / him / his P'unk Avenue | (215) 755-1330 | punkave.com

bkosborne commented 5 years ago

Anyone know of a workaround for an NPM-noob?

bkosborne commented 5 years ago

Okay I understand a bit better now. Babel needs to be run after browserify because browserify is what resolves all the module dependencies into a single file. Babel doesn't have access to them before that step.

I think just running babel again on the single file that browserify creates solves this. Babel also tried to create compact output since the file is so large, which I disabled in the babelrc file (see "compact" option). Then I was able to run uglifyjs without the errors.

bkosborne commented 5 years ago

More specifically, here is what I did:

That gets you dist/sanitize-html.min.js.

Note that I also needed polyfills for IE11 support, which the version of Babel that this package ships with does not automatically include. To fix that, before running the above commands, I added require('babel-polyfill'); to the very top of src/index.js.

boutell commented 5 years ago

Something like this has now been done in master, except I don't have the requiring of the polyfill in there yet.

(I am beginning to think that having a dist file for this module is a mistake, people probably have their own webpack build and they should clearly use it so we don't pull in polyfills twice and who knows what else. But my mixed feelings on using this module browser-side at all are well-known (: )

boutell commented 5 years ago

Want to do a PR for the polyfill part based on latest master?

kevin074 commented 5 years ago

Hi, I just want to add that the current scripts works again, IF you "mkdir dist" in the root directory.

boutell commented 5 years ago

A PR could be helpful, then.

On Tue, Oct 15, 2019 at 6:21 PM kevin074 notifications@github.com wrote:

Hi, I just want to add that the current scripts works again, IF you "mkdir dist" in the root directory.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/sanitize-html/issues/300?email_source=notifications&email_token=AAAH27KAUQB7ZPKPLWTEXVLQOY7ABA5CNFSM4IML7ZD2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBKNEOQ#issuecomment-542429754, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27JRLE2CSUQLE3ITRADQOY7ABANCNFSM4IML7ZDQ .

-- Chief Software Architect Apostrophe Technologies Pronouns: he / him / his

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.