Skip to content

harmonize npm pkgs part3 #4237

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

Merged
merged 9 commits into from
Nov 25, 2019
Merged

harmonize npm pkgs part3 #4237

merged 9 commits into from
Nov 25, 2019

Conversation

haraldschilly
Copy link
Contributor

@haraldschilly haraldschilly commented Nov 19, 2019

Description

further updating packages (after "take2" branch)


this section is informative (i.e. don't update to immutable@4 blindly!), but outdated

in particular, the more restrictive typing in util/sync needs to be checked. e.g. in one case I made the note:

is there anything to do when val is undefined? maybe delete the key?

sync tests pass, though

hmm, something is odd, when loading a jupyter notebook the hub and project hub use up a lot of memory, produce a lot of output, etc.


I've ditched the immutable js update and it's back at an "@3" version – except for the webapp, where I don't dare to downgrade it.

The changes I had to make to the sync lib are in this closed and never merged PR. They certainly introduce problems, which aren't picked up by the tests.

Testing Steps

Relevant Issues

Checklist:

  • No debugging console.log messages.
  • All new code is actually used.
  • Non-obvious code has some sort of comments.

Front end:

  • Restart at least one project and check its ~/.smc/local_hub/local_hub.log
  • Completely restart Webpack with ./w in /src
  • Completely restart the hub by killing and restarting ./start_hub.py in /src/dev/project
  • Screenshots if relevant.

@haraldschilly haraldschilly changed the title Npm harmonize pkgs part3 harmonize npm pkgs part3 Nov 19, 2019
@haraldschilly haraldschilly force-pushed the npm-harmonize-pkgs-part3 branch from a597f6b to 775639f Compare November 20, 2019 09:38
@haraldschilly haraldschilly force-pushed the npm-harmonize-pkgs-part3 branch from 775639f to ffd9077 Compare November 20, 2019 09:41
JQuery = require('jquery')
dom = new JSDOM("", { runScripts: "dangerously" })
_jQuery_cached = JQuery(dom.window);
cb(_jQuery_cached)
Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, cool!

@williamstein williamstein merged commit 3afdcb8 into master Nov 25, 2019
@williamstein
Copy link
Contributor

This (maybe?) breaks manage-* on kucalc. All the manage-* images crash like so:

[prod3.test] kucalc-prod3-ctl-ws:~/kucalc/cluster2/addons/manage> c logs manage-state                                                                          
Error: Cannot find module './cache'                                                                                                                            
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:636:15)                                                                                
    at Function.Module._load (internal/modules/cjs/loader.js:562:25)                                                                                           
    at Module.require (internal/modules/cjs/loader.js:692:17)                                                                                                  
    at require (internal/modules/cjs/helpers.js:25:18)                                                                                                         
    at Object.<anonymous> (/home/manage/src/node_modules/ajv/lib/ajv.js:5:13)                                                                                  
    at Module._compile (internal/modules/cjs/loader.js:778:30)                                                                                                 
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:789: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)    

It's possible though that that some other PR between 11/2 and now actually broke manage. I'm going to try to fix this.

@williamstein
Copy link
Contributor

williamstein commented Nov 25, 2019

Hmmm... OK, we use ts-node caching VERY heavily, and I would say CoCalc massively uses it for halfway decent performance. Without caching every single project startup (in cc-in-cc dev mode) requires compiling every single ts file and will take about one minute (every single time!). Probably a lot of other things we build are much slower. Also, all manage, hub, etc., services will start up MUCH more slowly.

ts-node completely eliminated caching support, due to a problem with how hashes are computed for dependent files: TypeStrong/ts-node#672

The current behavior is just to silently ignore requesting to use caching, e.g., where we write

require('ts-node').register({ cacheDirectory:... })

with current ts-node it just doesn't work at all. So that's not so good.

I'm going to push another patch that reverts all changes to the ts-node back to 1.5 years ago.

Probably the right solution is to set this TS_NODE_TRANSPILE_ONLY environment variable for all containers where we run typescript code in production. For development, I don't know what we should do (e.g., for projects when doing cc-in-cc dev -- do we want to have a special way to run them that doesn't just strip the typescript, but by default we do strip the typescript?)

See #4253

@williamstein
Copy link
Contributor

Rebuilding elsewhere got rid of the crashes, so that may have been a caching issue. However, the fact is we need caching, so we have to revert ts-node.

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

Successfully merging this pull request may close these issues.

2 participants