feat(jsii): enable source maps for declaration files #3521
Conversation
andrestone
force-pushed
the
feat/declaration-map
branch
from
e99f05a
to
8a65b3f
Compare
|
Hey @andrestone - can you elaborate on why you're making this change? Depending on the rationale (you might have thought into this a bit more than I have), I wonder if we should just always turn the flag on, instead of making it customizable? I doubt it's very expensive to have, and it's "strongly recommended" by the TypeScript documentation... |
|
Hi @RomainMuller, thanks for your attention on this. I think the only downside for having it always on is the fact that, depending on how developers bundle the JS package, the files might be included in the bundle, since they're emitted to In my specific case, it wouldn't really make a difference (as long as I'm getting the declaration maps). |
|
Sorry I missed your first question. This is basically to help navigate source files when developing against a JSII library's repo (e.g using |
|
Are these emitted as separate files? If so, they could be added to In any case, I don't think there is much harm in actually shipping the declarations... If the corresponding sources aren't present, IDE integrations would just fall back to the "usual" process... They could be seen as dead-weight... but I doubt that'd be very heavy, so maybe this can be ignored? |
|
Yes, they're separate files with the extension |
andrestone
force-pushed
the
feat/declaration-map
branch
from
8a65b3f
to
a7e2bdf
Compare
andrestone
changed the title
andrestone
force-pushed
the
feat/declaration-map
branch
from
71b44ad
to
88bf493
Compare
|
@RomainMuller I'm unsure if we rely on |
|
That's okay, but you should not check in all the |
andrestone
force-pushed
the
feat/declaration-map
branch
from
88bf493
to
53bdfb6
Compare
Of course! I think I checked the root Should be ok now. |
andrestone
force-pushed
the
feat/declaration-map
branch
from
53bdfb6
to
326e1fc
Compare
|
Hey @RomainMuller! Sorry for the ping, just wanted to make sure there isn't anything else I could do here. |
|
Hey @andrestone -- sorry I got caught up in other stuff and forgot to check back here... At this stage, this looks good to go for me. |
|
@Mergifyio update |
✅ Branch has been successfully updated |
|
Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it! |
|
Merging (with squash)... |
Instead of enabling declarations maps everywhere as was done in #3521, allow customers to define their desired source-map related configuration in the `jsii.tsc` stanza of `package.json`. This change in stance is motivated by how introduction of declarations map causes broad asset hash changes in consumer code, which effectively breaks many snapshot-based regression tests, and this feature should hence be opt-in.
Instead of enabling declarations maps everywhere as was done in #3521, allow customers to define their desired source-map related configuration in the `jsii.tsc` stanza of `package.json`. This change in stance is motivated by how introduction of declarations map causes broad asset hash changes in consumer code, which effectively breaks many snapshot-based regression tests, and this feature should hence be opt-in. --- By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license]. [Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
Generates declaration maps when compiling.
This is useful to help navigate source files when developing against a JSII library's repo (e.g using yarn link).
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.