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

[Draft] [WIP] [Recorder] Using import for types #7056

Closed

Conversation

HarshaNalluru
Copy link
Member

No description provided.

@HarshaNalluru HarshaNalluru changed the title [Recorder] Using import instead of require and rollup fixes [WIP] [Recorder] Using import instead of require and rollup fixes Jan 22, 2020
@HarshaNalluru HarshaNalluru changed the title [WIP] [Recorder] Using import instead of require and rollup fixes [Draft] [WIP] [Recorder] Using import instead of require and rollup fixes Jan 22, 2020
Copy link
Member

@xirzec xirzec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a couple thoughts

"dist-esm/test/*.spec.js",
"dist-esm/test/node/*.spec.js",
"dist-esm/src/index.js"
];
baseConfig.plugins.unshift(multiEntry());

// different output file
baseConfig.output.file = "dist-test/index.node.js";

// mark assert as external
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we update this comment since it's not just assert that is getting excluded now?

dont_print: true
});
if (!isBrowser()) {
import("nock").then((nock) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since import is async, I think record() should become an async function and then we can use await here.

if (err.code !== "EEXIST") throw err;
}
if (!isBrowser()) {
import("nock").then((nock) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same remark

@HarshaNalluru HarshaNalluru added Client This issue points to a problem in the data-plane of the library. test-utils-recorder Label for the issues related to the common recorder labels Feb 6, 2020
@HarshaNalluru HarshaNalluru changed the title [Draft] [WIP] [Recorder] Using import instead of require and rollup fixes [Draft] [WIP] [Recorder] Using import for types Feb 8, 2020
@ramya-rao-a
Copy link
Contributor

@HarshaNalluru, Is this PR still relevant? What is pending here to get the PR out from the draft stage?

@HarshaNalluru
Copy link
Member Author

HarshaNalluru commented Jun 22, 2020

I'll need to re-work since some of the changes here would be breaking, I'll check it out this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library. test-utils-recorder Label for the issues related to the common recorder
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants