Add SEA config to node.config.json

[marco-ippolito]

What is the problem this feature will solve?

--experimental-sea-config requires yet another config file, I think this can be added as namespace in the node.config.json rather than into another file.

What is the feature you are proposing to solve the problem?

We could merge the content of --experimental-sea-config into --experimental-config-file and add a new namespace in the JSON called sea.

What alternatives have you considered?

keep another config file

cc @RaisinTen @joyeecheung

[joyeecheung]

I think we can have both? Though it starts to get weird when the built SEA also reads from this config file for command line options, and also weird if it doesn't. The SEA config is not a runtime config and is not supposed to be in NODE_OPTIONS either. I think a more natural solution would be the other way around, to allow the CLI options to be nested into the sea config file in the format of node config (to be used when the SEA starts), instead of nesting SEA config into node config and now there is the question of when is this config used.

[marco-ippolito]

We could add a namespace for sea, which is different from the nodeOptions namespace. Since we now have a place to throw all different json config we could standardize it. We could keep both but imho longterm it makes more sense put everthing in the node.config.json then have a different json for each new feature

[marco-ippolito]

We couls add a namespace for sea, which is different from the nodeOptions namespace. Since we now have a place to throw all different json config we could standardize it. We could keep both but imho longterm it makes more sense put everthing in the node.config.json then have a different json for each new feature that needs one

[RaisinTen]

The current state of having a separate config file for SEAs seems fine because there doesn't seem to be any obvious problems in terms of maintenance or usage.

If we add the sea namespace to the node.config.json file, it would complicate where the SEA is supposed to read the node options from - the node options in the nested sea config or the top level nodeOptions property or both (like a fallback mechanism). Even if we document it, it's pretty easy for a user to get confused about this and have incorrect expectations about this.

[marco-ippolito]

Im talking about this:

{ "main": "/path/to/bundled/script.js",
 "output": "/path/to/write/the/generated/blob.blob",
 "disableExperimentalSEAWarning": true,
"useSnapshot": false,
"useCodeCache": true,
 "assets": { "a.dat": "/path/to/a.dat",
 "b.txt": "/path/to/b.txt" } 
} 

not sea flags

Being able to put this config in the node.config.json under sea namespace.
It should not be controversial.
And instead of reading it from --experimental-sea-config it can use node.config.json. this will also grant json schema.

[joyeecheung]

I would prefer that we keep --experimental-sea-config/--sea-config with just the top-level JSON configuration. It's fine to add a namespace to node.config.json if people want to merge it into one file, but we should not rip people away from the ability to create a more succinct config for readability. I don't think it's a problem to keep a shorter, more focused configuration for SEA blob preparation specifically, and it's actually better than using node.config.json for this than throwing everything in the same schema IMO, because it's not a runtime config for a Node.js application, but a configuration for the output blob, that will be eventually used to build a Node.js application, which may additionally need a different configuration for runtime. Keeping it separate helps reducing the confusion as @RaisinTen mentioned. I think for bigger projects, mixing build configs with runtime configs in one file would lead to more problems than it solves.

[marco-ippolito]

I'm fine with being able to keep the two separate.
The node.config.json is convienient because its loaded by default and it keeps everything consistent, with a json schema, without have to deal with multiple config files.
I assume you would be ok with a node.config.json like:

{
  "sea": {
    "main": "/path/to/bundled/script.js",
    "output": "/path/to/write/the/generated/blob.blob",
    "disableExperimentalSEAWarning": true,
    "useSnapshot": false,
    "useCodeCache": true,
    "assets": {
      "a.dat": "/path/to/a.dat",
      "b.txt": "/path/to/b.txt"
    }
  }
}

How do we deal when the config is in both? Which wins?
Or we can throw an error if --experimental-sea-config is specified and the config has a sea namespace

[joyeecheung]

The node.config.json is convienient because its loaded by default and it keeps everything consistent, with a json schema, without have to deal with multiple config files.

In terms of the SEA build config, I think being loaded by default can be weird, what would the work flow look like for this to be loaded by default?

echo '{ "sea": { "main": "hello.js", "output": "sea-prep.blob" } }' > node.config.json
node
cp node hello
npx postject hello NODE_SEA_BLOB sea-prep.blob --sentinel-fuse NODE_SEA_FUSE_fce680ab2cc467b6e072b8b5df1996b2
hello  # is this going to read from node.config.json again? what is that even suppose to do?

I don't think the sequence is more readable than a more explicit

echo '{ "main": "hello.js", "output": "sea-prep.blob" }' > sea-config.json
node --experimental-sea-config sea-config.json
cp node hello
npx postject hello NODE_SEA_BLOB sea-prep.blob --sentinel-fuse NODE_SEA_FUSE_fce680ab2cc467b6e072b8b5df1996b2
hello  # can read node.config.json, if there's one, which does not include SEA configs, but also weird to have one because that defeats the purpose of a single file.

[joyeecheung]

How do we deal when the config is in both? Which wins?

IMO the least confusing thing to do is to just not mix SEA config into the node config. Just keep things as is and add a new runtimeOptions field in the existing sea-config.json schema, which nests node config schema. Then it seem pretty intuitive that --experimental-sea-config is just used to configure what would be bundled into the final output. And technically supporting an additional node.config.json is probably not really a very useful thing for SEA at runtime because that sort of defeats the whole purpose of making it a single file.

[RaisinTen]

Agreed. For now, I think we should keep the SEA config separate. Let's see how the node.config.json file evolves and we can revisit this in the future if it makes sense.