Skip to content

fix(dlp): resolve CommonJS compatibility issues and stabilize test #4350

Open
angelcaamal wants to merge 3 commits into
mainfrom
fix-dlp-mime-esm
Open

fix(dlp): resolve CommonJS compatibility issues and stabilize test #4350
angelcaamal wants to merge 3 commits into
mainfrom
fix-dlp-mime-esm

Conversation

@angelcaamal

@angelcaamal angelcaamal commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Description

This PR fixes runtime errors in the DLP samples caused by dependency versions that exclusively support ESM, which are incompatible with the existing CommonJS architecture of these samples.

Changes included:

  • Module Compatibility Fix: Downgraded problematic dependencies to the latest versions that maintain full CommonJS support, resolving the "ERR_REQUIRE_ESM" and related module import crashes.
  • Test/Sample Fix (inspectWithCustomHotwords.js): Updated the hardcoded mock SSN from 222-22-2222 to 458-90-3124. The DLP API's ML model identifies repetitive/obvious test data as VERY_UNLIKELY. Since the sample is configured with minLikelihood: POSSIBLE, the test was failing to detect findings. Using a realistic sequence ensures the API returns the expected results in the test suite.

Fixes Internal: b/525822800

Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google.

Checklist

  • I have followed guidelines from CONTRIBUTING.MD and Samples Style Guide
  • Tests pass: npm test (see Testing)
  • Lint pass: npm run lint (see Style)
  • Required CI tests pass (see CI testing)
  • These samples need a new API enabled in testing projects to pass (let us know which ones)
  • These samples need a new/updated env vars in testing projects set to pass (let us know which ones)
  • This pull request is from a branch created directly off of GoogleCloudPlatform/nodejs-docs-samples. Not a fork.
  • This sample adds a new sample directory, and I updated the CODEOWNERS file with the codeowners for this sample
  • This sample adds a new sample directory, and I created GitHub Actions workflow for this sample
  • This sample adds a new Product API, and I updated the Blunderbuss issue/PR auto-assigner with the codeowners for this sample
  • Please merge this PR for me once it is approved

Note: Any check with (dev), (experimental), or (legacy) can be ignored and should not block your PR from merging (see CI testing).

@angelcaamal angelcaamal added api: dlp Issues related to the Sensitive Data Protection API. samples Issues that are directly related to samples. labels Jun 19, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request downgrades the mime dependency from version ^4.0.0 to ^3.0.0 in the devDependencies of dlp/package.json. There are no review comments, and I have no feedback to provide.

@angelcaamal angelcaamal changed the title fix(dlp): downgrade mime to v3 to restore CommonJS compatibility fix(dlp): resolve CommonJS compatibility issues and stabilize test Jun 19, 2026
@angelcaamal angelcaamal marked this pull request as ready for review June 19, 2026 19:24
@angelcaamal angelcaamal requested review from a team as code owners June 19, 2026 19:24

@Kef131 Kef131 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

@amcolin amcolin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Downgrading the dependency doesn't seem like the best practice in this situation. Could we explore the possibility of implementing dynamic imports instead?

// Remove this line
// const {assert} = require('chai');
let assert;

// And on the before 
before(async () => {
    // To bypass the ERR_REQUIRE_ESM restriction
    const chai = await import('chai');
    assert = chai.assert;

    projectId = await client.getProjectId();
  });

@angelcaamal

angelcaamal commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

@amcolin Thanks for your feedback. While that possibility exists, the way libraries are imported across all samples in this repository is using require() (CommonJS module system); we currently do not have support for ESM.
For context, these libraries were updated by Mend Renovate, and the update broke backward compatibility (There are no vulnerabilities for previous versions).

@angelcaamal angelcaamal requested a review from amcolin June 19, 2026 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: dlp Issues related to the Sensitive Data Protection API. samples Issues that are directly related to samples.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants