Take "target" from base tsconfig when present

isaacs / tshy

Other

869 stars 17 forks source link

Take "target" from base tsconfig when present #55

Closed timovv closed 6 months ago

timovv commented 6 months ago

Following up on #54 with the suggested fix. It works for us when we try to build in our Azure SDK repo, but unsure if it would cause issues for folks who have their own tsconfig.json with target unset and are relying on tshy to set the target?

isaacs commented 6 months ago

Hm, yeah, the upgrade story here is a little bit concerning, and it seems like a small thing to do a semver major bump for (though that's not out of the question, of course). Maybe we could have tshy check to see if target is set in a pre-existing tsconfig.json, and add to .tshy/build.json if it's missing?

mpodwysocki commented 6 months ago

@isaacs I agree that we should keep it as ES2022 or whatever default we have now if it is not present in the base tsconfig. @timovv Thoughts?

mpodwysocki commented 6 months ago

@isaacs one way we could also do it not to break people is to have it opt in like other configuration items like selfLink and other options.

timovv commented 6 months ago

I don't have a strong opinion here, other than that I agree it seems like a small thing to do a breaking change for. Out of the options, I lean towards setting the target only if it's not set in the tsconfig -- I think that's the most intuitive behavior we'd be able to get without breaking. Let me have a go at implementing that.

Otherwise if going with an extra configuration option for this I would say let's just have the option be "target", defaulting to es2022, and continue to ignore the tsconfig field entirely as we do today.

timovv commented 6 months ago

OK, I have changed this so the behavior is:

If target is specified in the base tsconfig.json (this includes the recommended config if it is created), then build.json doesn't specify a target.
Otherwise, target es2022 is added to the build.json

isaacs commented 6 months ago

This looks great. Minor style nit, I can get to it if you don't before it lands. Thanks!

timovv commented 6 months ago

@isaacs: just wanted to check if there's anything outstanding we can help with to get this in? We have some customers waiting on the fix, we're happy to vendor it in the interim if need be to unblock them but would be great to see this upstream :)