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

build static assets #4280

Closed
JrCs opened this issue Jun 20, 2018 · 15 comments · Fixed by #4311
Closed

build static assets #4280

JrCs opened this issue Jun 20, 2018 · 15 comments · Fixed by #4311
Labels
Support Support question

Comments

@JrCs
Copy link

JrCs commented Jun 20, 2018

I have modified the file readthedocs/core/static-src/core/js/readthedocs-doc-embed.js and use the gulp buildcommand to regenerate the static assets:

./node_modules/.bin/gulp build
[18:11:14] Using gulpfile ~/workspace/upstreams/readthedocs.org/gulpfile.js
[18:11:14] Starting 'build'...
[18:11:14] Building source files
(node:4130) [DEP0022] DeprecationWarning: os.tmpDir() is deprecated. Use os.tmpdir() instead.

The problem is that the readthedocs/core/static/core/js/readthedocs-doc-embed.js is not regenerated.

@stsewd stsewd added the Support Support question label Jun 20, 2018
@stsewd
Copy link
Member

stsewd commented Jun 20, 2018

I think this was something to do with the node version.

@JrCs
Copy link
Author

JrCs commented Jun 20, 2018

Any idea of what version of nodejs to use ?

@stsewd
Copy link
Member

stsewd commented Jun 20, 2018

$ node --version
v10.0.0
$ npm --version
5.6.0

@ericholscher
Copy link
Member

I have the same issue, and have had trouble debugging it, so I'd love to see if we find a fix for this.

@JrCs
Copy link
Author

JrCs commented Jun 21, 2018

After some investigations, the readthedocs/core/static/core/js/readthedocs-doc-embed.js is generated if minify is turn off but some embed js file are not add in it.
In the git log i see that @agjohnson was commit new version of the ``readthedocs/core/static/core/js/readthedocs-doc-embed.js` file. Perhaps he can give us some hints how he do it ?

@stsewd
Copy link
Member

stsewd commented Jun 21, 2018

I'm able to build the minified js version with the above node and npm versions btw.

@JrCs
Copy link
Author

JrCs commented Jun 22, 2018

@stsewd is the readthedocs-doc-embed.js correctly minify with all the inclusions in it ?
If yes do you start from a fresh gulp/bower install ?
If yes can you give us informations about on what OS do you made the gulp build ?
I have try in an debian container and it doens't works for the readthedocs-doc-embed.js file.

@humitos
Copy link
Member

humitos commented Jun 22, 2018

I had some similar problems and I used node 9.2.1 and worked. I don't know the root cause of the issue though.

nodenv helped me to setup a different node version in my system.

@JrCs
Copy link
Author

JrCs commented Jun 25, 2018

Thanks @humitos, but i have test with node 9.2.1 and it doesn't work for the readthedocs-doc-embed.js file. I think the problem is not the version of nodejs but a problem of dependencies that are downloaded.

@stsewd
Copy link
Member

stsewd commented Jun 25, 2018

I haven't re-downloaded the dependencies in a long time, so probably the problem is there 🤔

@agjohnson
Copy link
Contributor

FWIW, I've tested with 9.2.1 and fresh dependencies and can rebuild without issue.

@davidfischer
Copy link
Contributor

I am pretty certain this has to do with one of the dependencies and not with node/npm itself. If I clear all the dependencies and rebuild, I see this issue.

rm -rf node_modules/ bower_components/
npm i && bower install
gulp build

I tried both node v8.11.3 (the latest LTS as of this comment) and node v6.9.4 with npm v6.1.0 and I see the issue with both node versions.

I think the DeprecationWarning: os.tmpDir() is deprecated. Use os.tmpdir() instead is not the real issue. If I drop down to node v6.9.4 I don't see the deprecation warning but readthedocs-doc-embed.js is still not regenerated.

@davidfischer
Copy link
Contributor

After some investigations, the readthedocs/core/static/core/js/readthedocs-doc-embed.js is generated if minify is turn off

I see this as well and it is odd

@ericholscher
Copy link
Member

I get this exception when I run uglifyjs directly on the file. It works properly when called with other files, so guessing that might be it?

-> ./node_modules/.bin/uglifyjs readthedocs/core/static/core/js/readthedocs-doc-embed.js

/Users/eric/projects/readthedocs.org/node_modules/uglify-js/lib/parse.js:204
    throw new JS_Parse_Error(message, line, col, pos);
    ^
Error
    at new JS_Parse_Error (/Users/eric/projects/readthedocs.org/node_modules/uglify-js/lib/parse.js:196:18)
    at js_error (/Users/eric/projects/readthedocs.org/node_modules/uglify-js/lib/parse.js:204:11)
    at croak (/Users/eric/projects/readthedocs.org/node_modules/uglify-js/lib/parse.js:636:9)
    at token_error (/Users/eric/projects/readthedocs.org/node_modules/uglify-js/lib/parse.js:644:9)
    at expect_token (/Users/eric/projects/readthedocs.org/node_modules/uglify-js/lib/parse.js:657:9)
    at expect (/Users/eric/projects/readthedocs.org/node_modules/uglify-js/lib/parse.js:660:36)
    at /Users/eric/projects/readthedocs.org/node_modules/uglify-js/lib/parse.js:1160:44
    at /Users/eric/projects/readthedocs.org/node_modules/uglify-js/lib/parse.js:683:24
    at expr_atom (/Users/eric/projects/readthedocs.org/node_modules/uglify-js/lib/parse.js:1117:35)
    at maybe_unary (/Users/eric/projects/readthedocs.org/node_modules/uglify-js/lib/parse.js:1287:19)

@davidfischer
Copy link
Contributor

davidfischer commented Jun 28, 2018

I believe I have a fix for this issue #4311 if any of you folks would be willing to test it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Support Support question
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants