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

feat: inline cjs index #1113

Merged
merged 2 commits into from
Jan 11, 2024
Merged

feat: inline cjs index #1113

merged 2 commits into from
Jan 11, 2024

Conversation

kuhe
Copy link
Contributor

@kuhe kuhe commented Dec 14, 2023

This PR adds a script that can be run for any package.
It is a variation of the standard dist-cjs or Node.js build.

The transform inlines the contents of the package into the package's dist-cjs/index.js file using esbuild.
This inlining does not include external packages, only the source code of the package itself. The purpose of this is to reduce file resolution time in application initialization for Node.js.

Additionally, any files which have package metadata or filename metadata indicating they are variant files used by React Native are preserved as external to the inlined index.

@kuhe kuhe force-pushed the experiment/inliner branch from 5aa94fc to 242d342 Compare December 22, 2023 21:03
@kuhe kuhe force-pushed the experiment/inliner branch 14 times, most recently from 280717d to e9aac90 Compare January 9, 2024 15:26
@kuhe kuhe marked this pull request as ready for review January 9, 2024 15:26
@kuhe kuhe requested review from a team as code owners January 9, 2024 15:26
@kuhe kuhe requested a review from kstich January 9, 2024 15:26
@kuhe kuhe force-pushed the experiment/inliner branch 2 times, most recently from 67073da to fa2ce26 Compare January 9, 2024 17:34
@kuhe kuhe force-pushed the experiment/inliner branch 5 times, most recently from 279257f to 36aba17 Compare January 11, 2024 18:34
@kuhe kuhe force-pushed the experiment/inliner branch from 36aba17 to efbf641 Compare January 11, 2024 18:54
@kuhe
Copy link
Contributor Author

kuhe commented Jan 11, 2024

additional revisions:

  • fix cases where react-native entry point is string
  • fix for cyclical transitive variant files
  • fix for multiple require path replacement of variant files

@kuhe kuhe merged commit 1ae1f4c into smithy-lang:main Jan 11, 2024
7 checks passed
@kuhe kuhe deleted the experiment/inliner branch January 11, 2024 21:16
@kuhe kuhe mentioned this pull request Jan 16, 2024
4 tasks
trentm added a commit to trentm/opentelemetry-js-contrib that referenced this pull request Jan 25, 2024
…dk-v3 instrumentation

As of smithy-lang/smithy-typescript#1146
(details at smithy-lang/smithy-typescript#1113)
the CommonJS export for many (all?) `@smithy/*` packages is now an esbuild
bundle -- all in `dist-cjs/index.js`. That means that subfile patching like this
no longer works:

```js
    const v3SmithyMiddlewareStackFile = new InstrumentationNodeModuleFile(
      '@smithy/middleware-stack/dist-cjs/MiddlewareStack.js',
      ['>=1.0.1'],
      this.patchV3ConstructStack.bind(this),
      this.unpatchV3ConstructStack.bind(this)
    );
```

In our case this breaks as of `@smithy/[email protected]` released
2024-01-17T22:26:42.432Z.  This is considered a non-breaking change, so the
dependency ranges for earlier released versions of `@smithy/smithy-client` will
pick this up.

A fix is to change the `@smithy/middleware-stack` patching to be on the top-level
module exports. Because the `constructStack` field is only available as a getter
we cannot use shimmer (InstrumentationBase._wrap). Instead this returns a new
`moduleExports` object with a new getter that shims the call.
trentm added a commit to open-telemetry/opentelemetry-js-contrib that referenced this pull request Feb 5, 2024
…dk-v3 instrumentation (#1913)

As of smithy-lang/smithy-typescript#1146
(details at smithy-lang/smithy-typescript#1113)
the CommonJS export for many (all?) `@smithy/*` packages is now an esbuild
bundle -- all in `dist-cjs/index.js`. That means that subfile patching like this
no longer works:

```js
    const v3SmithyMiddlewareStackFile = new InstrumentationNodeModuleFile(
      '@smithy/middleware-stack/dist-cjs/MiddlewareStack.js',
      ['>=1.0.1'],
      this.patchV3ConstructStack.bind(this),
      this.unpatchV3ConstructStack.bind(this)
    );
```

In our case this breaks as of `@smithy/[email protected]` released
2024-01-17T22:26:42.432Z.  This is considered a non-breaking change, so the
dependency ranges for earlier released versions of `@smithy/smithy-client` will
pick this up.

A fix is to change the `@smithy/middleware-stack` patching to be on the top-level
module exports. Because the `constructStack` field is only available as a getter
we cannot use shimmer (InstrumentationBase._wrap). Instead this returns a new
`moduleExports` object with a new getter that shims the call

This PR also updates .tav.yml to reduce the number of aws-sdk package versions tested.
HenryCorn pushed a commit to HenryCorn/opentelemetry-js-contrib that referenced this pull request Feb 6, 2024
…dk-v3 instrumentation (open-telemetry#1913)

As of smithy-lang/smithy-typescript#1146
(details at smithy-lang/smithy-typescript#1113)
the CommonJS export for many (all?) `@smithy/*` packages is now an esbuild
bundle -- all in `dist-cjs/index.js`. That means that subfile patching like this
no longer works:

```js
    const v3SmithyMiddlewareStackFile = new InstrumentationNodeModuleFile(
      '@smithy/middleware-stack/dist-cjs/MiddlewareStack.js',
      ['>=1.0.1'],
      this.patchV3ConstructStack.bind(this),
      this.unpatchV3ConstructStack.bind(this)
    );
```

In our case this breaks as of `@smithy/[email protected]` released
2024-01-17T22:26:42.432Z.  This is considered a non-breaking change, so the
dependency ranges for earlier released versions of `@smithy/smithy-client` will
pick this up.

A fix is to change the `@smithy/middleware-stack` patching to be on the top-level
module exports. Because the `constructStack` field is only available as a getter
we cannot use shimmer (InstrumentationBase._wrap). Instead this returns a new
`moduleExports` object with a new getter that shims the call

This PR also updates .tav.yml to reduce the number of aws-sdk package versions tested.
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.

3 participants