Support asconfig.json

[jtenner]

I gave this change a lot of thought before I even started coding it, but I bet there are multiple ways to improve this pull request.

• I've read the contributing guidelines

Todo:

• Utilize new merge mechanic
• Support asconfig.include? (string[])
• Testing (still more that needs to be done
• Asinit changes
  • add default asconfig
  • modify existing asconfig

[dcodeIO]

Regarding the algorithm, I'm wondering if this can be integrated into options.parse by adding one more argument, an object of previously parsed options. Now, if the object is given, it simply wouldn't overwrite existing options with defaults? So, one starts with an empty object {} that will be fully populated with either given options or defaults since there are no existing ones on it, and get a fully populated object. That object can now be given to parse again, but this time, since all keys exist, would not apply defaults again and only overwrite with given options?

Edit: Oh, wait, this operates on args, not opts. But perhaps there is a way to put all of this into util/options.js, and let it reuse the relevant parts?

[jtenner]

It looks like enabled and disabled logic is more complicated than I thought. Because the cli provides a string array, it needs to be treated as a set of immutable flags by the time it gets passed to resolve the asconfig.

What's more difficult is that each nested asconfig level needs to handle enable and disable flags before they get overridden by the cli.

I am proposing that the enable and disable flags are removed from the MVP.

[jtenner]

Restarting my efforts here. Need to think this through a little bit more.

[jtenner]

Progress:

• Added support for the ability of asconfig to specify entry points
  • This causes the compiler to add entry points from extended configurations too
  • Duplicate entry points are calculated and removed before they are passed to the compiler
  • Entry points must be relative to the asconfig, (e.g. ./asconfig.json can point to ./assembly/index.ts, ignoring any provided basedir)

We still need to add some testing projects in the repo to verify this functionality works. This pull request is becoming much larger than I previously thought.

[jtenner]

@dcodeIO If perhaps someone would like to extend multiple configs to obtain their entry points, this becomes impossible because extends is currently defined as a string. Perhaps this field is best designed as a string[].

Example, I have a library that would like to obtain lib entries from two distinct projects:

{
  "targets": {
    "release": {
       "optimizeLevel": 3,
       "shrinkLevel": 2
     }
  },
  "extends": [
    "./node_modules/@assemblyscript/node/asconfig.json",
    "./node_modules/some-utf8-lib/asconfig.json"
  ]
}

Now both sets of lib entries are added to the compilation.

Thoughts?

[dcodeIO]

Looks like this is getting dangerously close to substituting what imports would do, but given that the same isn't achievable with just imports when it comes to additional entries or similar, I'm not necessarily opposed to allow extends to be an array as an option, as long as it can also be a string which I'd assume to be the more common case?

[jtenner]

Why not both? Sounds good to me.

[jtenner]

I've switched to supporting an MVP in this pull request again. Just taking a bit of time to figure out how to do testing. I suspect I'll be ready for review in a week.

[dcodeIO]

Problem with the wat file seems to be here. Moving that below the args.outFile check so whateverFile is populated should fix it.

[jtenner]

Problem with the wat file seems to be here. Moving that below the args.outFile check so whateverFile is populated should fix it.

I'm sorry, what do you mean by "that?"

[dcodeIO]

I mean the linked declaration of hasOutput. If its value is set after deriving the format of outFile, the behavior will be correct. If you think that should be a separate PR, this PR here can be fixed by using --binaryFile instead of --outFile. Or you can just fix the cause here if you prefer. In general this PR looks pretty good now with the last comments addressed :)

[jtenner]

Oh I see. I thought this was my fault 😅 . Yeah. Let's pull that out into a separate PR. Just wondering if everything here is appropriately addressed now?

[dcodeIO]

Merging master should fix the wat problem.

[jtenner]

Please let me know if I can help finish this. I am very excited to refactor my cli completely to account for asconfig.

[dcodeIO]

Looks good to merge once CI passed, unless there are remaining objections?

[jtenner]

I wish I could extend multiple configurations, but I really like the elegance of this algorithm using a while loop like I designed it more. Thank you for your feedback!

[jtenner]

Looks like it's all set.

[github-actions]

🎉 This PR is included in version 0.14.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[dcodeIO]

Turned out that path.relative does all sorts of funny things in the browser, like stripping away the first character of entry files, but only in bundles and otherwise hard to reproduce. Not particular a fault of this PR, yet browser builds are still heavily searching for that ominous odule.ts instead of functioning properly.

[jtenner]

Right. Should we add a test to make sure browserified bundles work?