Fix API exposing threejs properties by diegoteran · Pull Request #4502 · google/model-viewer (original) (raw)

@diegoteran

Removes ThreeJS' TextureFilter leak in our API

@diegoteran

@diegoteran

@diegoteran

@diegoteran

@diegoteran

We should not expose threejs from our public api.

@diegoteran

@diegoteran

elalish

Filter.LinearMipmapLinear,
Filter.NearestMipmapLinear,
Filter.LinearMipmapLinear
const minFilterValues: Array<MinificationTextureFilter> = [

Choose a reason for hiding this comment

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

Can you just use minificationToMinFilter instead of creating this extra list?

const magFilterValues: Array = [Filter.Nearest, Filter.Linear];
return (value: unknown): value is MagFilter =>
magFilterValues.indexOf(value as MagFilter) > -1;
const magFilterValues: Array =

Choose a reason for hiding this comment

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

likewise.

@@ -1,5 +1,3 @@
import {TextureFilter} from 'three';

Choose a reason for hiding this comment

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

👍

@diegoteran

elalish

Choose a reason for hiding this comment

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

Thanks!

JL-Vidinoti pushed a commit to vidinoti/model-viewer that referenced this pull request

Apr 22, 2024

@diegoteran @JL-Vidinoti

We should not expose threejs from our public api.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})