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

Avoid importing Cesium3DTileset in Picking.js #8261

Closed
puckey opened this issue Oct 7, 2019 · 5 comments · Fixed by #8532
Closed

Avoid importing Cesium3DTileset in Picking.js #8261

puckey opened this issue Oct 7, 2019 · 5 comments · Fixed by #8532

Comments

@puckey
Copy link
Contributor

puckey commented Oct 7, 2019

Since updating to the latest version of Cesium, I noticed my project's bundle size had grown 85% by over 850kb minified.

I tracked down a large chunk of it to a single instanceof Cesium3DTileset use here: https://github.com/AnalyticalGraphicsInc/cesium/blob/d26ccfea38dcf4623cf5da3faca141913c64cfe1/Source/Scene/Picking.js#L549 causing the bundle size to grow by 549 kB, while I do not use Cesium3DTileset in my project.

Perhaps instead of using instanceof we could have a static type enum property to check against? For example if (primitive.type === Type.Cesium3DTileset)

@puckey puckey changed the title Avoid importing Cesium3DTile* in Picking.js Avoid importing Cesium3DTileset in Picking.js Oct 7, 2019
@hpinkos
Copy link
Contributor

hpinkos commented Oct 7, 2019

@lilleyse please make this a priority before the October release

@lilleyse
Copy link
Contributor

lilleyse commented Oct 7, 2019

@puckey All the code in Picking.js used to exist in Scene.js. Was Scene.js previously included in your project's bundle?

Perhaps instead of using instanceof we could have a static type enum property to check against?

Yes, something like that might work.

@hpinkos
Copy link
Contributor

hpinkos commented Oct 7, 2019

Ahhh, I do see that Cesium3DTileset has been required by Scene for a few releases. It probably shouldn't be though for apps not using combined Cesium. I downgraded this to priority - high.

@puckey were you using the latest version of cesium before you upgraded to the 1.62 release?

@puckey
Copy link
Contributor Author

puckey commented Oct 8, 2019

I wasn't using the very latest version before – I must have been a few releases behind.

@puckey puckey closed this as completed Oct 8, 2019
@hpinkos
Copy link
Contributor

hpinkos commented Oct 8, 2019

Gotcha, no problem @puckey
I'm going to keep this open because this is absolutely something we should fix. There's no need to pull in the entire 3D tiles source into this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants