[WIP] Create subpackages for internal-ish code #2351

steveluscher · 2024-03-20T19:58:16Z

Summary

The entire problem that this PR tries to solve is this.

Retain the simplicity of being able to export * from '@solana/errors' in packages/library/
Prevent things like safeCaptureStackTrace() from being exported from packages/library/

The steps taken to that end were:

Change moduleResolution everywhere to NodeNext. This enables subpackage exports
Add .js extensions everywhere to satisfy that resolution regime
Create a subpackage export in @solana/errors for all of the stuff that we want to share internally but not to export from packages/library/

Now any package in the @solana scope can use @solana/errors/internal but we can still enjoy the simplicity of export * from '@solana/errors' in packages/library/

changeset-bot · 2024-03-20T19:58:19Z

⚠️ No Changeset found

Latest commit: 39c0653

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

steveluscher · 2024-03-20T19:58:32Z

[WIP] Create subpackages for internal-ish code #2351 👈
master

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @steveluscher and the rest of your teammates on Graphite

steveluscher · 2024-03-20T20:06:21Z

packages/build-scripts/package.json

@@ -3,7 +3,6 @@
    "version": "0.0.0",
    "private": true,
    "files": [
-        "add-js-extension-to-types.mjs",


Nuked this, in favour of actually adding the .js extension everywhere.

steveluscher · 2024-03-20T20:06:41Z

packages/build-scripts/getBaseConfig.ts

@@ -25,7 +25,7 @@ export function getBaseConfig(platform: Platform, formats: Format[], _options: O
                              __REACTNATIVE__: `${platform === 'native'}`,
                              __VERSION__: `"${env.npm_package_version}"`,
                          },
-                          entry: [`./src/index.ts`],
+                          entry: ['./src/index.ts', './src/__internal.ts'],


This makes the build pick up this __internal.ts entrypoint if it exists.

steveluscher · 2024-03-20T20:07:03Z

packages/build-scripts/getBaseConfig.ts

@@ -70,6 +70,7 @@ export function getBaseConfig(platform: Platform, formats: Format[], _options: O
                          platform: platform === 'node' ? 'node' : 'browser',
                          pure: ['process'],
                          sourcemap: format !== 'iife' || isDebugBuild,
+                          splitting: false,


This prevents tsup from doing fancy bundle splitting between entrypoints.

steveluscher · 2024-03-20T20:08:16Z

packages/errors/src/__internal.ts

@@ -0,0 +1 @@
+export * from './stack-trace.js';


Here's that new ‘internal stuff’ entrypoint.

steveluscher · 2024-03-20T20:09:34Z

packages/rpc/src/rpc-integer-overflow-error.ts

@@ -1,4 +1,5 @@
-import { safeCaptureStackTrace, SOLANA_ERROR__RPC__INTEGER_OVERFLOW, SolanaError } from '@solana/errors';
+import { SOLANA_ERROR__RPC__INTEGER_OVERFLOW, SolanaError } from '@solana/errors';
+import { safeCaptureStackTrace } from '@solana/errors/internal';


Here's an example of making use of the internal subpackage.

steveluscher · 2024-03-20T20:10:06Z

packages/tsconfig/base.json

+        "module": "NodeNext",
+        "moduleResolution": "NodeNext",


This is the fancy new resolution strategy that allows all of this to work.

steveluscher · 2024-03-20T20:10:50Z

packages/test-config/jest-unit.config.common.ts

+    moduleNameMapper: {
+        // Strip `.js` from imports
+        // https://github.com/swc-project/jest/issues/64#issuecomment-1029753225
+        '^(\\.{1,2}/.*)\\.js$': '$1',
+    },


Of course Jest doesn't know any of this, so we have to re-strip the .js extensions.

lorisleiva · 2024-03-20T21:40:47Z

Ugh it's so sad because I do love the end result but I cannot bear having to use .js extensions on .ts files. And the fact that we then have to strip them for Jest is the icing on the cake.

Is there no way we can do this as a build step instead?

steveluscher · 2024-03-20T21:42:45Z

Is there no way we can do this as a build step instead?

Do what; add the extensions? No, because once you change to "moduleResolution": "NodeNext" it's gg.

lorisleiva · 2024-03-21T10:36:15Z

Well, adding "moduleResolution": "NodeNext" to our tsconfig changes how things are compiled, right?

What I'm saying is, instead of using the module resolution, can we not replicate the changes that this does at compile time to make subpackages work properly?

What exactly is NodeNext changing in our compiled code that makes subpackages possible?

steveluscher · 2024-03-21T16:52:24Z

What exactly is NodeNext changing in our compiled code that makes subpackages possible?

I think subpackage imports just straight up don't work unless moduleResolution is node16 or higher.

With moduleResolution: NodeNext:

With moduleResolution: Node:

lorisleiva · 2024-03-21T16:57:23Z

Oh I see, as a consumer of our own API, we need NodeNext to import sub-packages. No matter how we export them. I'm happy to disagree and commit if everyone agrees that the benefits are worth it. .js extensions on .ts files do make me want to cry though.

steveluscher commented Mar 20, 2024

View reviewed changes

steveluscher force-pushed the 03-20-_wip_create_subpackages_for_internal-ish_code branch from 2f890a3 to e057dc3 Compare March 20, 2024 21:58

[WIP] Create subpackages for internal-ish code

39c0653

steveluscher force-pushed the 03-20-_wip_create_subpackages_for_internal-ish_code branch from e057dc3 to 39c0653 Compare March 21, 2024 18:43

steveluscher added the do-not-close Add this tag to exempt an issue/PR from being closed by the stalebot label Apr 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Create subpackages for internal-ish code #2351

[WIP] Create subpackages for internal-ish code #2351

steveluscher commented Mar 20, 2024 •

edited

changeset-bot bot commented Mar 20, 2024 •

edited

steveluscher commented Mar 20, 2024

steveluscher Mar 20, 2024

steveluscher Mar 20, 2024

steveluscher Mar 20, 2024

steveluscher Mar 20, 2024

steveluscher Mar 20, 2024

steveluscher Mar 20, 2024

steveluscher Mar 20, 2024

lorisleiva commented Mar 20, 2024

steveluscher commented Mar 20, 2024

lorisleiva commented Mar 21, 2024

steveluscher commented Mar 21, 2024

lorisleiva commented Mar 21, 2024

[WIP] Create subpackages for internal-ish code #2351

Are you sure you want to change the base?

[WIP] Create subpackages for internal-ish code #2351

Conversation

steveluscher commented Mar 20, 2024 • edited

Summary

changeset-bot bot commented Mar 20, 2024 • edited

⚠️ No Changeset found

steveluscher commented Mar 20, 2024

steveluscher Mar 20, 2024

Choose a reason for hiding this comment

steveluscher Mar 20, 2024

Choose a reason for hiding this comment

steveluscher Mar 20, 2024

Choose a reason for hiding this comment

steveluscher Mar 20, 2024

Choose a reason for hiding this comment

steveluscher Mar 20, 2024

Choose a reason for hiding this comment

steveluscher Mar 20, 2024

Choose a reason for hiding this comment

steveluscher Mar 20, 2024

Choose a reason for hiding this comment

lorisleiva commented Mar 20, 2024

steveluscher commented Mar 20, 2024

lorisleiva commented Mar 21, 2024

steveluscher commented Mar 21, 2024

lorisleiva commented Mar 21, 2024

steveluscher commented Mar 20, 2024 •

edited

changeset-bot bot commented Mar 20, 2024 •

edited