Reuse 'is' function across fiber and test-renderer packages.

[andersonleite]

What this PR is doing The is function is identical both at fiber and test-renderer packages. We also have 2 identical tests in each package.

Changes

• move the is function to the shared folder so the function can be reused across both packages (similar to what is already done for asyncUtils)
• create the tests folder under shared and move one test for the is function there.
• update the references to the is function to use it from the shared folder _ Thanks!

[andersonleite]

Although the tests are passing locally, the build throws this message: error @react-three/fiber all relative imports in a package should only import modules inside of their package directory but "src/core/renderer.ts" is importing "../../../shared/is"

So probably it's not possible to reuse the is function across packages. From The Preconstruct documentation: "Preconstruct requires that all imports in a package must either packages that are specified as dependencies or peerDependencies or modules inside of a package's directory. This is so that Preconstruct can know that all source files are in the package directory which is necessary to know for the way Preconstruct does TypeScript declaration generation and because when you do this, it is generally a mistake."

[joshuaellis]

I believe if we put shared with a pkg json like @react-three/shared you'll be able to share via importing that way? I don't think we'd need to release the package for it to work, i think 🤔, but this would be a good avenue to explore to avoid duplication.

[andersonleite]

Thanks for the directions @joshuaellis

I've put shared in a pkg json as you suggested: @react-three/shared Both fiber and test-rendered are importing it as dependency and use is and asyncUtils from it and all the tests pass when running locally. I'm using npm link to make the @react-three/shared package available to both.

I believe the build error now is because @react-three/shared is not a published npm package. Any suggestion on how to proceed from here?

🎁 error @react-three/shared packages must have at least one entrypoint, this package has no entrypoints

[joshuaellis]

I believe the build error now is because @react-three/shared is not a published npm package. Any suggestion on how to proceed from here?

see inline comment above!

[codesandbox-ci[bot]]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 5742bde3dbd763dba1333228a1649b13053abbac:

Sandbox	Source
example	Configuration

[CodyJasonBennett]

As to answer your comment above, we don't want users to install an internal dependency, so this should be bundled into the final build as a dependency.

[joshuaellis]

I'll run through this later today, I need to check when it's used in rttr i believe it will be a required dep in r3f though.

[andersonleite]

Thanks @CodyJasonBennett and @joshuaellis

Just to summarize where we are at this moment: After adding the references bellow to the packages/shared/package.json as suggested, Preconstruct builds successfully without the need of installing as a public published package.

  "main": "dist/react-three-shared.cjs.js",
  "module": "dist/react-three-shared.esm.js",
  "types": "dist/react-three-shared.cjs.d.ts",

Then, we need some reference to /shared both in r3f and rttr. I'm doing that with import { is } from '@react-three/shared'. For this to work, I'm adding it as dependency (or peerDependency) on the package.json, what will give the following structure after the build:

@react-three

fiber shared test-renderer

I'll be taking a look again today, let me know if you have more thoughts on it.

[joshuaellis]

it should be a dep of rttr which i have made said change. I've just realised we'd have to start releasing this package as an npm module I believe? Which means the namespacing isn't great, because there are a few @react-three/* packages. So shared isn't actually shared 🤔

[CodyJasonBennett]

I know with Lerna we could use symlinks to locally include private packages in a namespace.

Maybe there's an equivalent with Preconstruct.

[andersonleite]

I know with Lerna we could use symlinks to locally include private packages in a namespace.

Maybe there's an equivalent with Preconstruct.

As I could verify, Lerna has --hoist option. (link) I could not find something similar to this on Preconstruct

[CodyJasonBennett]

Can we point this to v8?

Do note that is is a bit sensitive as it's used to diff within the reconciler. This duplication might've been intentional, but we can give it a try.

[drcmda]

i have removed the duplicated and made is.equ more generic, it can now compare arrays and objects shallow or by reference, which should now suit all usecases.

[CodyJasonBennett]

I ended up removing is from RTTR in #2465. It was only needed to null-check an array argument and I don't foresee it being useful elsewhere -- diffing is internal to R3F.