search

iTwin / imodels-clients

Monorepo for iModels API clients
MIT License
6 stars 1 forks source link

[Abandoned] Include source maps in published packages #91

Closed roluk closed 2 years ago

roluk commented 2 years ago

All JavaScript files that are published to npmjs from this repository declare that they have accompanying source maps when in reality they are missing. This results in tools like source-map-loader issuing runtime errors when a dev server encounters missing source map references.

This PR edits .npmignore files to allow generated .js.map files to be included in all published packages.

austeja-bentley commented 2 years ago

@roluk Is there a particular reason why you need the .js.map files published apart from the mentioned tool crash? I would assume the better fix for this particular situation is just to not include the source map reference in published .js files so that the tools would not expect them.

roluk commented 2 years ago

@roluk Is there a particular reason why you need the .js.map files published apart from the mentioned tool crash? I would assume the better fix for this particular situation is just to not include the source map reference in published .js files so that the tools would not expect them.

Both solutions are viable, but I fail to understand how not shipping source maps can be a better option.

I don't see myself ever using these source maps, however, this doesn't mean nobody will find them useful.

austeja-bentley commented 2 years ago

@roluk Is there a particular reason why you need the .js.map files published apart from the mentioned tool crash? I would assume the better fix for this particular situation is just to not include the source map reference in published .js files so that the tools would not expect them.

Both solutions are viable, but I fail to understand how not shipping source maps can be a better option.

I don't see myself ever using these source maps, however, this doesn't mean nobody will find them useful.

Currently we do not use inline source maps (that are generated using inlineSourceMap flag) so the source maps that we currently have do not bring value unless we publish sources. As of now do not plan to publish source files.

roluk commented 2 years ago

Currently we do not use inline source maps (that are generated using inlineSourceMap flag) so the source maps that we currently have do not bring value unless we publish sources. As of now do not plan to publish source files.

You don't need to additionally ship source files, source maps can contain the original source content themselves.

Speaking of which, I have inspected the source map output and found that sources were not included, so I fixed that in 9a6f7d07f10f43ea7a67e5f698433aa737494bd8.

austeja-bentley commented 2 years ago

/azp run

azure-pipelines[bot] commented 2 years ago
No pipelines are associated with this pull request.