AngelMunoz / Finny

A cross-platform tool for unbundled front-end development that doesn't depend on Node or requires you to install a complex toolchain
https://perla-docs.web.app/
MIT License
132 stars 11 forks source link

Environment variables #85

Closed nojaf closed 1 year ago

nojaf commented 2 years ago

Is your feature request related to a problem? Please describe. I'm currently in a situation where I need to use an API key. Is there a way to use environment variables so I don't need to use the key hardcoded?

Describe the solution you'd like Something like process.env.exports or whatever it was again in Node.

Describe alternatives you've considered I can understand this is a tricky request, I can't think of much right now.

Additional context Add any other context or screenshots about the feature request here.

AngelMunoz commented 2 years ago

I'd like to solve this in a more standard way

As the consumer I would expect you to do something like :

import {API_KEY, CLIENT_TOKEN} from '/env.js'
open Fable.Core
open Fable.Core.JsInterop

let apiKey = import "API_KEY" "/env.js"
let CLIENT_TOKEN = importMember "/env.js"

from Perla's side it should take the responsibility to read .env files and serve that env.js file at dev time, after a build it should also output that next to the final bundle.

That way you can either serve that dynamically (when you have control of the server where the static content is at) at /env.js or grab the generated one from the locally present .env files and

nojaf commented 2 years ago

Well, I won't have a .env file in my CI build so I would prefer both options to be supported. I could live with the import {API_KEY, CLIENT_TOKEN} from '/env.js' proposal. During a perla build I would appreciate no secrets being written on disk. Just to avoid if the build failed, the file not be deleted from the disk.

AngelMunoz commented 2 years ago

the problem with process.env is that, it only works with node stuff the browser has no process global, that I've seen similar tooling do is to use "import.meta.*", the meta on the import.meta has the advantage of being also part of the module standards, but I'm not sure how to implement it properly

In the case of vite/snowpack the approach is very similar they have a import.meta.* regex and replace the contents of the file when serving it

with this source code

const env = import.meta.env;

console.log(env?.MODE);
console.log(import.meta.env);

console.log(env);
export function setupCounter(element) {
  let counter = 0;
  const setCounter = (count) => {
    counter = count;
    element.innerHTML = `count is ${counter}`;
  };
  element.addEventListener("click", () => setCounter(++counter));
  setCounter(0);
}

at runtime we get this output from the dev server

image

but the regex can break with valid code

image

I don't really want to mess with file content too much until I have something in place that can operate on files (the plugin stuff ;-: ) but in the meantime I think the /env.js option is more sensible

I'll keep looking into it

nojaf commented 2 years ago

As I said:

I could live with the import {API_KEY, CLIENT_TOKEN} from '/env.js' proposal.

Meaning, really it's ok 😸.

My main concern would be that the secrets are written to disk during a build. Because the solution of importing the data from a special module would require that file to exist for esbuild I imagine.

AngelMunoz commented 2 years ago

As I said:

I could live with the import {API_KEY, CLIENT_TOKEN} from '/env.js' proposal.

Meaning, really it's ok 😸.

Yeah, I was just adding context for future references (in case I forget why I did what I did, happens quite often)

My main concern would be that the secrets are written to disk during a build. Because the solution of importing the data from a special module would require that file to exist for esbuild I imagine.

Normally yes, but what I did there is to use the "/env.js" (configurable, the docs are up if you want to check them out) import as an external for the esbuild compilation and esbuild will ignore that, if you opt in then you can emit that env file

In any case esbuild will ignore that file and we'll try to provide the correct external based on the devServer.envPath

AngelMunoz commented 2 years ago

This is out in v0.24.0

nojaf commented 2 years ago

Hey, that was quick. Thank you. Building didn't work for me:

Perla:Build: Starting esbuild with pid: [22708]
Perla:Build: No Entrypoints for CSS found in index.html
✘ [ERROR] Could not resolve "../../../../../env.js"

                                                       out/App.js:5:29:
                                                                             5 │ import { FOO as FOO_1 } from "../../../../../env.js";
                                                                                                                                              ╵                              ~~~~~~~~~~~~~~~~~~~~~~~

                                                                                                                                                                                                    1 error
                                                                                                                                                                                                           Perla: There was an error running this command
CliWrap.Exceptions.CommandExecutionException: Underlying process reported a non-zero exit code (1).

Command:
  C:\Users\nojaf\.nuget\packages\perla\0.24.0\tools\net6.0\any\package\esbuild.exe C:\Users\nojaf\Projects\thunder\out\App.js --external:firebase/app --external:firebase/firestore --external:react --external:react-dom --external:react-firehooks/firestore --bundle --target=es2020 --loader:.png=file 
--loader:.svg=file --loader:.woff=file --loader:.woff2=file --minify --format=esm --outdir=./dist\out

You can suppress this validation by calling `WithValidation(CommandResultValidation.None)` on the command.

I've added:

  "devServer": {
    ...,
    "enableEnv": true,
    "envPath": "/env.js"
  },

to my config.

Feedback there: this seems weird that you need to enable it, given you've introduced the PERLA_ prefix requirement. The prefix is fine and I would just enable all of this by default. Only introduce the enableEnv when somebody really really wants to turn this off.

Lastly, I mentioned that I didn't want the file to be on disk, but actually, that doesn't really make much sense. All your keys will be either in your bundle or somewhere else in plain text in your output. Snowpack places this in a separate file, not sure how vite or webpack do it, but disregard my previous concern.

AngelMunoz commented 2 years ago

Hey there, let me see what's going on

Could not resolve "../../../../../env.js"

how are you importing this file?

the default is /env.js so any imports you make to that file, have to actually be anchored to the root of the server rather than ../../env.js

In this case I'm using the default behavior of browser imports, a rooted path can be imported from anywhere in the application so no need for relative paths

Feedback there: this seems weird that you need to enable it, given you've introduced the PERLA_ prefix requirement. The prefix is fine and I would just enable all of this by default. Only introduce the enableEnv when somebody really really wants to turn this off.

It is enabled by default no need to add that in the configuration file

Lastly, I mentioned that I didn't want the file to be on disk, but actually, that doesn't really make much sense. All your keys will be either in your bundle or somewhere else in plain text in your output. Snowpack places this in a separate file, not sure how vite or webpack do it, but disregard my previous concern.

Yeah it was a little bit odd but no worries you can output to disk but you need to enable that in the build node

{
  "build": {
    "emitEnvFile": true
  }
}

not sure how vite or webpack do it

I think they go through every file and if there's any kind of import.meta.env.* string in the source code they replace or re-assign to that variable from whatever it was on the system

AngelMunoz commented 2 years ago

I have to fix a couple of mistakes from my side but if you agree I can also enable emitEnvFile by default

nojaf commented 2 years ago

I can also enable emitEnvFile by default

Yeah, I would really remove all the configuration, to be honest. Less maintenance for you and there is no real evidence yet this can't be opinionated 😸.

AngelMunoz commented 2 years ago

v0.24.1 has been released, it should emit by default

nojaf commented 2 years ago

I'm still having the same problem with 0.24.1 I'm afraid.

In my F# I have:

let FOO = import "FOO" "/env.js"

printfn "secret foo:%s" FOO

I removed everything from my perla.jsonc. Running

pwsh> $env:PERLA_FOO = "BAR"; dotnet perla b

Still leads to

Fable compilation finished in 2542ms

Perla:Build: Starting esbuild with pid: [16528]
Perla:Build: No Entrypoints for CSS found in index.html
✘ [ERROR] Could not resolve "../../../../../env.js"

                                                       out/App.js:5:29:
                                                                             5 │ import { FOO as FOO_1 } from "../../../../../env.js";
                                                                                                                                              ╵                              ~~~~~~~~~~~~~~~~~~~~~~~

                                                                                                                                                                                                    1 error
                                                                                                                                                                                                           Perla: There was an error running this command
CliWrap.Exceptions.CommandExecutionException: Underlying process reported a non-zero exit code (1).

Command:
  C:\Users\nojaf\.nuget\packages\perla\0.24.1\tools\net6.0\any\package\esbuild.exe C:\Users\nojaf\Projects\myproject\out\App.js --external:/env.js --external:firebase/app --external:firebase/firestore --external:react --external:react-dom --external:react-firehooks/firestore --bundle --target=es2020 
--loader:.png=file --loader:.svg=file --loader:.woff=file --loader:.woff2=file --minify --format=esm --outdir=./dist\out

You can suppress this validation by calling `WithValidation(CommandResultValidation.None)` on the command.
  at async Task<CommandResult> CliWrap.Command.AttachAsync(ProcessEx process, CancellationToken cancellationToken) in Command.cs:480
  at async Task<CommandResult> CliWrap.Command.AttachAsync(ProcessEx process, CancellationToken cancellationToken) in Command.cs:486
  at async void Perla.Lib.Build.buildFiles@225.MoveNext() in Build.fs:243
  at async void Perla.Lib.Build.execBuild@296.MoveNext() in Build.fs:335
  at Ply<FSharpResult<void, Exception>> Program.main@56-6.Invoke(void unitVar0)
  at async void Ply.TplPrimitives.ContinuationStateMachine`1.System-Runtime-CompilerServices-IAsyncStateMachine-MoveNext()
  at Ply<FSharpResult<int, Exception>> Program.main@56-8.Invoke(void unitVar0)
  at async Ply<u> Ply.TplPrimitives.AwaitableContinuation`3.Invoke(void r)

I'm on Windows if that is relevant.

nojaf commented 2 years ago

Aha, Fable appears to compile this as such:

import { FOO as FOO_1 } from "../../../../../env.js";

This probably still is /env.js on Unix.

AngelMunoz commented 2 years ago

hmmm... This looks weird indeed

I will try to repro this,

I did try it with a typescript file I don't believe fable should be resolving the files like that

AngelMunoz commented 2 years ago

I just opened this issue https://github.com/fable-compiler/Fable/issues/3146 on the fable side

I'll let you know what happens there

in the case that the fable folks aren't able to fix this, we should be able to emit this file before the esbuild process starts so it has a chance to figure out where the file is, but... that could break things at runtime...

If that ends up being the case I'll see what can be done

nojaf commented 2 years ago

Hello, I was able to work around the fable limitation using dynamic imports in JavaScript. The next problem I'm facing is that my app is being deployed to a subfolder. Something like https://domain.com/subfolder. The /env.js isn't working out because of that. Is there any option to set this folder during build?

nojaf commented 2 years ago

Actually, my dynamic import workaround code is probably the problem:

    JsInterop.importDynamic "/env.js"
    |> Promise.map (fun env -> env?API_ROOT)

I'm not sure how it works but esbuild probably won't interfere with this code. Suggestions?

AngelMunoz commented 2 years ago

oh that's right fable is ignoring dynamic imports at the moment

Is there any option to set this folder during build?

For the build phase specifically no, it will take the default value used from the devServer config

 "devServer": {
    "envPath": "/path/to/env.js"
  },

that being said, you would need to also change your imports to "/path/to/env.js"

AngelMunoz commented 2 years ago

I'm not sure how it works but esbuild probably won't interfere with this code. Suggestions?

we add the "envPath" value to the "externals" so esbuild doesn't mess with it and leaves the import as is

nojaf commented 2 years ago

I'm also looking for a base config option. Updating the envPath is fine but it is a bit unnecessary for my local development. I'm pointing my envPath to /<Repo name>/env.js and referencing it as such in my code, but it really is a deploy concern.

AngelMunoz commented 1 year ago

I don't expect fable to fix the path issues any time soon, so this will be addressed in #105 and #106 so I will close this issue as the original request has been out for a while