Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(npm packages): remove test files and tsconfig.json from npm tar… #133

Merged
merged 3 commits into from
Feb 28, 2018

Conversation

jgravois
Copy link
Contributor

@jgravois jgravois commented Feb 28, 2018

…balls

since we are distributing built types, UMDs, ES6 modules (with types) and common JS modules for node, it seems safe to remove the tsconfig.json entirely, especially since its incomplete.

there's no need to specify "README.md" or "package.json" or whatever is listed as "main". they are always included. this change drops the tests too to further shrink the bundles.

this will also solve problems like unintentionally shipping cruft like this:
https://unpkg.com/@esri/arcgis-rest-request@1.0.3/.rpt2_cache/

AFFECTS PACKAGES:
@esri/arcgis-rest-auth
@esri/arcgis-rest-common-types
@esri/arcgis-rest-feature-service
@esri/arcgis-rest-geocoder
@esri/arcgis-rest-groups
@esri/arcgis-rest-items
@esri/arcgis-rest-request

ISSUES CLOSED: #132

…balls

AFFECTS PACKAGES:
@esri/arcgis-rest-auth
@esri/arcgis-rest-common-types
@esri/arcgis-rest-feature-service
@esri/arcgis-rest-geocoder
@esri/arcgis-rest-groups
@esri/arcgis-rest-items
@esri/arcgis-rest-request

ISSUES CLOSED: #132
Copy link
Contributor

@patrickarlt patrickarlt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jgravois do we even need the src/** files?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 37940e7 on patch132 into 2a30d22 on master.

@jgravois
Copy link
Contributor Author

jgravois commented Feb 28, 2018

do we even need the src/** files?

good question.

@JeffJacobson (or any other TypeScript developer), is it sufficient when a lib you depend on only exposes ES6 modules and types and doesn't include raw .ts files?

https://unpkg.com/@esri/arcgis-rest-request@1.0.3/src/
https://unpkg.com/@esri/arcgis-rest-request@1.0.3/dist/esm/

@Esri Esri deleted a comment from coveralls Feb 28, 2018
@tomwayson
Copy link
Member

tomwayson commented Feb 28, 2018

I am far from a "TypeScript developer", but, FWIW, I have been including them in other TS projects like esri-loader, but I think that's a mistake.

I'm almost certain that they are not needed because main, module, types, browser etc all point to files under dist, the only way a consuming TS app could access the src files would be import { request } from './node_modules/@esri/arcgis-rest-request/src/request.ts, and even then unless they were using the same tsconfig as us, they could have compile errors.

I say we drop src too. If we do, I'll plan to do the same in esri-loader and my other TS packages.

@patrickarlt
Copy link
Contributor

@jgravois TypeScript uses the files under dist like @tomwayson said. This is why we publish all the .d.ts files in the esm folder.

@jgravois
Copy link
Contributor Author

I say we drop /src

done and done.

@patrickarlt patrickarlt merged commit 232b863 into master Feb 28, 2018
@jgravois jgravois deleted the patch132 branch May 14, 2018 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants