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

Glasser/package bundler #6556

Closed
wants to merge 3 commits into from
Closed

Glasser/package bundler #6556

wants to merge 3 commits into from

Conversation

glasser
Copy link
Member

@glasser glasser commented Jun 10, 2022

No description provided.

@glasser glasser force-pushed the glasser/package-bundler branch 4 times, most recently from 2f02d88 to 800d17d Compare June 10, 2022 00:43
@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 10, 2022

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 8aa0eee:

Sandbox Source
Apollo Server Typescript Configuration
Apollo Server Configuration

This reverts commit 0b58585.

This leaves the fix of adding cors and body-parser to server's deps and
leaves the removal of a TODO(AS4) about this.

The build process for this package is completely different from a normal
TS package, so keeping it together seems best. This seems like it'll
make setting up ESM builds easier.
This would give us two things: publishing as ESM in addition to CJS, and
the ability to have some symbols come from "deep imports" that aren't
loaded automatically. It replaces some hacks like runtime requires in
src/plugin/index.ts, and adds things like "you don't need to load
express unless you actually import from the standalone directory".

A directory with an index.ts in it is an entry point. The main rule is:
you can import arbitrarily within your own entry point, but you can't do
a "deep" import of a specific file or directory inside another entry
point. Note that this means that any code that needs to be accessible
from multiple entry points but not externally importable should be in an
entry point whose name contains "internal" so that folks understand they
shouldn't rely on its existence.

This is coming close to working. Some issues I'm running into include:

- TS directive emission isn't working because of the change to make
  packages/server/tsconfig.json only cover package.json. But maybe that
  can be reverted somehow.
- How do we actually enforce that bad deep imports don't happen?
- Is there still any point to ever running tsc emitting output? (Like
  for Jest maybe?) Should we ditch dist entirely? Not sure how though.
- We're using the "copy and rewrite package.json into output dir"
  approach because that way the deep imports are
  `@apollo/server/standalone` not `@apollo/server/bundled/standalone`. I
  already found out how to get codesandbox CI to publish from the right
  directory but we have to do the same for changesets probably.
- I did some refactors to make ApolloServer.ts use runtime import() to
  pull in the default-installed plugins but in CJS mode that still seems
  to include the plugin's code in the CJS file --- it won't actually be
  run (including its import of protobuf stuff) until needed at least.

Let's figure out more next week.
@glasser
Copy link
Member Author

glasser commented Jun 17, 2022

Closing in favor of #6580

@glasser glasser closed this Jun 17, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant