dumberjs / dumber-gist

A lightweight online IDE to write JS SPA prototypes in your own GitHub gists.
https://gist.dumber.app
MIT License
28 stars 2 forks source link

environment variables to set up domains #33

Closed aegenet closed 4 years ago

aegenet commented 4 years ago

Hello,

This pull request is intended to allow you to set up all the domains used by gist.dumber via environment variables :

Why ? It will allow to deploy this great application more easily via Docker. Or simply to be able to deploy a gist.dumber "beta".

One breaking change: "client-service-worker" must be built, otherwise I can't inject the HOST_NAMES.

Then you need to build client-service-worker code. Run

cd client-service-worker/
npm i # or yarn or pnpm i
npm run build

(Besides, I added "cross-env" library for build compatibility on Windows).

Usage

DUMBER_DOMAIN=beta-dumber.app && npm run build

Thanks

PS : I'm also working on another PR to connect to NPM (for private projects).

3cp commented 4 years ago

Thx, it looks pretty good to me! Although I don't need this feature, it doesn't hurt.

Obviously I need time to test this through, it's a fairly big diff.

3cp commented 4 years ago

Before you create the other PR to connect to NPM, could you explain it a bit? I am wondering do we need that feature in this project, that sounds very private.

aegenet commented 4 years ago

Thx, it looks pretty good to me! Although I don't need this feature, it doesn't hurt.

Obviously I need time to test this through, it's a fairly big diff.

* [ ]  I need to update `copy-to-server.sh` and `deploy.sh` to match the `client-service-worker/dist`.

Careful, I have already changed the nginx configuration files

location / {
        ---root   /home/huocp/dumber-gist/client-service-worker;
        +++root   /home/huocp/dumber-gist/client-service-worker/dist;
aegenet commented 4 years ago

Before you create the other PR to connect to NPM, could you explain it a bit? I am wondering do we need that feature in this project, that sounds very private.

A number of companies, individual creators or associations use npm with private projects. Either with the official version (npmjs) or clones made by the community (verdaccio).

To retrieve these projects it is mandatory to be logged in (this highlights an issue with jsdelivr).

It's not a feature for everyone, thus it's possible to create a fork and maintain the fork, but I think it's a loss for the community.

A feature is still a feature, you can use it or not, but I totally understand your position regarding the maintainability of the project.

3cp commented 4 years ago

Are you talking about private npm registry or packages to build the project? Not runtime deps in user apps (projects in dumber gist)?

aegenet commented 4 years ago

I'm talking about projects in dumber gist. The idea is to allow the user to connect to npm (client side), then the worker (ever client side) can used the credentials.

3cp commented 4 years ago

Oh, I see. That's actually a useful feature being asked before.

I didn't invest time on npm registry apis. Let me know how it goes. I expect official npm does not allow cors requests, it means I wound need a backend to proxy the request which is a possible architecture bottleneck (I mean for a free product with limited budget on deployment).

Also, to save user credentials, this project needs proper user management in backend, otherwise you will save sensitive user info in client side localStorage or indexdb, that's not acceptable. Proper user management is also out of scope of this project, as it is designed to run on github gists to avoid user management. A new project is probably needed to meet the requirement of private npm packages. I got few friends suggesting various ideas on full featured online IDE, a commercial product. But so far, I have no plan on that yet, but it's on my radar.

aegenet commented 4 years ago

Also, to save user credentials, this project needs proper user management in backend, otherwise you will save sensitive user info in client side localStorage or indexdb, that's not acceptable.

I don't totally agree with that. If npm authentication returned a signed token (like JWT), locked on the domain, we wouldn't be any different from any other web app in the world. Afterwards, yes, it's always better when you can expose as less data as possible, but ATM wouldn't be a blocking point (just my opinion).

Let's try.

3cp commented 4 years ago

Sure, as I said, I don't know how npm credentials works. Token is not credentials btw, it is created by exchanging credentials. If npm has oauth like github, credentials is not a thing to worry. But I expect cors is a real trouble.

3cp commented 4 years ago

You reminded me to check my npm access tokens. It looks like the tokens are not scoped to anything, a true secret. But my npm account is not a paid account, don't know what feature available for paid users.

aegenet commented 4 years ago

Don't change this message. The embedded browser detects this type of message and show readable hint to end user.

Hum, ok in "browser-frame", I see... but the message is not correct. The user may think it's just a cache problem when the version doesn't exist :-/.

Since I fixed the missing .catch(reject), the errors show up nicely in the built-in browser. Like : npm package "bignumber" was not found with requested version: "3".

3cp commented 4 years ago

The message was correct, if version does not exist, it wouldn't hit this api, it will fail resolve step first.

The only reason here is jsdelivr has not synced the version yet.

3cp commented 4 years ago

Sorry, I think missed the context. I will double check tomorrow. There maybe another place (in my mind) raising jsdelivr out of sync error in the code base.

3cp commented 4 years ago

Many thanks! It will take me few days to find time to finish off #35.

3cp commented 4 years ago

I made few changes. You would need to slightly adjust your local setup.

  1. renamed env var JSDELIVR_DATA_URL to JSDELIVR_DATA_DOMAIN, in order to be consistent with the other two env var DUMBER_DOMAIN and JSDELIVR_CDN_DOMAIN.

  2. removed gulpfile from client-service-worker, use client/gulpfile to update the single file in client-service-worker.

    • only one npm run build command to build them all.
    • As a result, nginx conf files are restored to previous version.
  3. remove process.env.NPM_URL because you did not use it anyway. It only adds confusion. It is for the other feature you are going to work on, not here. Pls use a consistent env name NPM_REGISTRY_DOMAIN when you work on it.

FYI I could not switch from registry.npmjs.cf to data.jsdelivr.com because jsdelivr doesn't provide enough meta data (it's much simpler than the full npm registry meta data). Details in #35