-
Notifications
You must be signed in to change notification settings - Fork 3.4k
chore: package @packages/network
as an independent bundle
#32633
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
Open
AtofStryker
wants to merge
3
commits into
develop
Choose a base branch
from
chore/network-bundle
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+534
−309
Conversation
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
cypress
|
Project |
cypress
|
Branch Review |
chore/network-bundle
|
Run status |
|
Run duration | 19m 57s |
Commit |
|
Committer | Bill Glesias |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
2
|
|
13
|
|
1102
|
|
4
|
|
26689
|
View all changes introduced in this branch ↗︎ |
Warning
Partial Report: The results for the Application Quality reports may be incomplete.
UI Coverage
45.11%
|
|
---|---|
|
186
|
|
157
|
Accessibility
97.96%
|
|
---|---|
|
4 critical
8 serious
2 moderate
2 minor
|
|
101
|
Tests for review
cypress/e2e/studio/studio.cy.ts • 2 failed tests • app-e2e
Test | Artifacts | |
---|---|---|
Cypress Studio > updates an existing test with assertions |
Test Replay
Screenshots
|
|
Cypress Studio > creates a new test from an empty spec |
Test Replay
Screenshots
|
commands/location.cy.js • 1 flaky test • 5x-driver-electron
Test | Artifacts | |
---|---|---|
... > does not log an additional log on failure |
Test Replay
|
issues/28527.cy.ts • 1 flaky test • 5x-driver-electron
Test | Artifacts | |
---|---|---|
issue 28527 > fails and then retries and verifies about:blank is not displayed |
Test Replay
Screenshots
|
e2e/origin/config_env.cy.ts • 1 flaky test • 5x-driver-inject-document-domain-chrome
Test | Artifacts | |
---|---|---|
cy.origin- Cypress.config() > serializable > overwrites different values in secondary if one exists in the primary |
Test Replay
|
commands/location.cy.js • 1 flaky test • 5x-driver-chrome
Test | Artifacts | |
---|---|---|
... > does not log an additional log on failure |
Test Replay
|
e2e/origin/config_env.cy.ts • 1 flaky test • 5x-driver-chrome
Test | Artifacts | |
---|---|---|
cy.origin- Cypress.config() > serializable > overwrites different values in secondary if one exists in the primary |
Test Replay
|
The first 5 flaky specs are shown, see all 13 specs in Cypress Cloud.
21c3259
to
2b03c2c
Compare
a6ac039
to
eab0745
Compare
187cfd7
to
267df5b
Compare
… network-tools is expected to be used in simple environments, where network is intended to be used in the node context. additionally, makes these packages bundable and removes the ts-node entrypoint to make ESM possible.
267df5b
to
e9f5df0
Compare
@packages/network
as an independent bundle@packages/network
as an independent bundle
3 tasks
9ede409
to
fbda21d
Compare
fbda21d
to
91201d8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Additional details
This PR refactors
@packages/network
into two packages,@packages/network
and@packages/network-tools
.Why are we doing this?
The refactor was prompted by the effort to make every package an independent bundle, where packages are built themselves and needed functions/modules are imported through the main package entrypoint and not importing the source directly. This means that:
and would need to do
Will no longer work as the the the source files are not compiled along side the module and are not shipped with the module. It may work in the monorepo, but it will FAIL when you attempt to build the binary. This means that the function needs to be imported from the package directly and not from a source file
This will change over time as we are able to update our TypeScript targets, but currently because we are bound to
moduleResolution:'node'
, we can't leverage export paths, similar to how we do in the CLI. Eventually, when our internal packages are ESM-only, this type of pattern will be much less of a concern, but until we get there we need to adhere to this type of compiling/module building.As a positive side effect, it makes individually type checking individual packages much easier as we can ship declaration types with the package, which is much better respected by
skipLibCheck
in thetsconfig.json
vs importing directly from source, which means the package importing from another module has to completely TypeCheck the other module.Right now, as we are in a grey area between
sinon
/mocha/chai
andvitest
, importing fromsrc
is usally fine in a testing context because the files are present. They are also much easier to mock insinon
then from a distributed package becausesinon
can't mock ES6 classes effectively. The only mechanism to do this is to stub the prototype, which is not ideal.The new structure
@packages/network-tools
only contains the cors, uri, and document domain utilities that are used commonly throughout the repo. This will likely include some of the cookie utils that are used in the server in the future to avoid circular dependencies between a given package and the server@packages/network-tools
only contains utilities and can run in either the browser OR the node context.@packages/network
as a true distributed package contains a lot of node code, which means it can't run in the browser unless we polyfill the libraries needed or change them. The frontend modules only need the non-node utilities, hence the refactor.With the refactor, I tried to remove bluebird references where possible and use
async
/await
. Now that the types of changed for how the package is built, I needed to fix/add types in some areas. A couple types are failing in the server in thev8-snapshot
task and to be honest, why that is currently is not clear to me, so those are handled with ats-ignore
.Steps to test
How has the user experience changed?
PR Tasks
cypress-documentation
?type definitions
?Note
Introduces @packages/network-tools for shared browser/node networking utils and rebuilds @packages/network as a CJS/ESM bundle, updating imports and refactoring code (incl. async/await) across the repo and CI.
@packages/network-tools
: exportscors
,uri
,document-domain-injection
, andtypes
; adds CJS/ESM builds, tsconfig, tests.@packages/network
: removes browser-focused utils, adds CJS/ESM build targets; refactorsagent
,connect
,allow-destroy
, client certs, and exports (incl.strictAgent
).@packages/network/lib/*
with@packages/network-tools
inconfig
,driver
(origin validation, location),proxy
(request/response middleware, buffers, cookies), andserver
(cloud APIs, routes, controllers, remote_states, ensure-url, server-base).@packages/network-tools
/@packages/network
built before binary checks.network
andnetwork-tools
as completed.Written by Cursor Bugbot for commit 91201d8. This will update automatically on new commits. Configure here.