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

set-value is not built in the artifact after node.js 18 and need to manually add it as a dependency #4064

Open
ananzh opened this issue May 18, 2023 · 0 comments

Comments

@ananzh
Copy link
Member

ananzh commented May 18, 2023

Description

After bumping to Node.js 18 and making various changes (https://github.com/AMoo-Miki/OpenSearch-Dashboards/tree/node-18-webpack-4), we found an interesting issue. When we run yarn build-platform --linux --skip-os-packages --release, set-value package is not built into the artifact though we didn't make any direct changes around this package.

This package is not shown in any package.json but in yarn.lock. It is a sub dependency package but directly called here

ubuntu@ip-172-31-55-237:~/OpenSearch-Dashboards$ yarn why set-value
yarn why v1.22.19
[1/4] Why do we have the module "set-value"...?
[2/4] Initialising dependency graph...
warning Resolution field "typescript@4.0.2" is incompatible with requested version "typescript@~4.5.2"
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "set-value@2.0.1"
info Reasons this module exists
   - "_project_#braces#snapdragon#base#cache-base" depends on it
   - Hoisted from "_project_#braces#snapdragon#base#cache-base#set-value"
   - Hoisted from "_project_#braces#snapdragon#base#cache-base#union-value#set-value"
info Disk size without dependencies: "20KB"
info Disk size with unique dependencies: "148KB"
info Disk size with transitive dependencies: "168KB"
info Number of shared dependencies: 5
=> Found "@types/globby#set-value@2.0.1"
info Reasons this module exists
   - "_project_#@types#globby#fast-glob#micromatch#snapdragon#base#cache-base" depends on it
   - Hoisted from "_project_#@types#globby#fast-glob#micromatch#snapdragon#base#cache-base#set-value"
   - Hoisted from "_project_#@types#globby#fast-glob#micromatch#snapdragon#base#cache-base#union-value#set-value"
   - in the nohoist list ["/_project_/**/@types/*","/_project_/**/@types/*/**","/_project_/**/grunt-*","/_project_/**/grunt-*/**","/_project_/@elastic/eui/rehype-react","/_project_/@elastic/eui/remark-rehype","/_project_/@elastic/eui/remark-rehype/**"]
info Disk size without dependencies: "68KB"
info Disk size with unique dependencies: "196KB"
info Disk size with transitive dependencies: "216KB"
info Number of shared dependencies: 5
=> Found "@osd/optimizer#set-value@2.0.1"
info Reasons this module exists
   - "_project_#@osd#optimizer#@types#watchpack#chokidar#braces#snapdragon#base#cache-base" depends on it
   - Hoisted from "_project_#@osd#optimizer#@types#watchpack#chokidar#braces#snapdragon#base#cache-base#set-value"
   - Hoisted from "_project_#@osd#optimizer#@types#watchpack#chokidar#braces#snapdragon#base#cache-base#union-value#set-value"
   - in the nohoist list ["/_project_/**/@types/*","/_project_/**/@types/*/**","/_project_/**/grunt-*","/_project_/**/grunt-*/**","/_project_/@elastic/eui/rehype-react","/_project_/@elastic/eui/remark-rehype","/_project_/@elastic/eui/remark-rehype/**"]
info Disk size without dependencies: "20KB"
info Disk size with unique dependencies: "148KB"
info Disk size with transitive dependencies: "168KB"
info Number of shared dependencies: 5
=> Found "@osd/pm#set-value@2.0.1"
info Reasons this module exists
   - "_project_#@osd#pm#@types#globby#fast-glob#micromatch#snapdragon#base#cache-base" depends on it
   - Hoisted from "_project_#@osd#pm#@types#globby#fast-glob#micromatch#snapdragon#base#cache-base#set-value"
   - Hoisted from "_project_#@osd#pm#@types#globby#fast-glob#micromatch#snapdragon#base#cache-base#union-value#set-value"
   - in the nohoist list ["/_project_/**/@types/*","/_project_/**/@types/*/**","/_project_/**/grunt-*","/_project_/**/grunt-*/**","/_project_/@elastic/eui/rehype-react","/_project_/@elastic/eui/remark-rehype","/_project_/@elastic/eui/remark-rehype/**"]
info Disk size without dependencies: "68KB"
info Disk size with unique dependencies: "196KB"
info Disk size with transitive dependencies: "216KB"
info Number of shared dependencies: 5
Done in 0.98s.

Among these references, only @osd/optimizer is used as a dependency in @osd/plugin-helpers. For other references and other usages of @osd/optimizer, they are all dev dependencies.

Understanding on InstallDependencies task

When we run yarn build-platform --linux --skip-os-packages --release, packages are built by InstallDependencies task

export const InstallDependencies: Task = {

export const InstallDependencies: Task = {
  description: 'Installing node_modules, including production builds of packages',

  async run(config, log, build) {
    const project = await Project.fromPath(build.resolvePath());

    await project.installDependencies({
      extraArgs: [
        '--production',
        '--ignore-optional',
        '--frozen-lockfile',
        '--prefer-offline',
        '--no-bin-links',
      ],
    });
  },
};

Here--production flag will cause yarn to install only the dependencies listed under "dependencies" in package.json, skipping "devDependencies". set-value is not explicitly declared in the dependencies or devDependencies sections of any package.json, but is present in the yarn.lock file. When the --production flag is set during installation, yarn only installs packages that are listed under the dependencies section in package.json, not the devDependencies. However, it does install sub dependencies that are required by the packages in dependencies. So set-value is sub-dependency of @osd/optimizer which is a dependency of @osd/plugin-helpers. I think that is why we build it into artifact.

Initial Analysis

From function wise this function is responsible for install dependencies, and I tried to add logs:

export async function installInDir(directory: string, extraArgs: string[] = []) {
  const options = ['install', '--non-interactive', ...extraArgs];

  console.log(`Installing dependencies in directory: ${directory}`);
  console.log(`Running command: ${YARN_EXEC} ${options.join(' ')}`);

  // We pass the mutex flag to ensure only one instance of yarn runs at any
  // given time (e.g. to avoid conflicts).
  await spawn(YARN_EXEC, options, {
    cwd: directory,
  });

  console.log('Dependencies installed.');
}

But I am not able to see logs print out. Then I start to use other packages to narrow down the issue.

Since we changed globby, I picked is-glob which is also a sub dependency of a dev dependency @types/globby and only shown in yarn.lock. is-glob has no issue in both node 18 branch and main. To reproduce , could modify https://github.com/opensearch-project/OpenSearch-Dashboards/blob/c5058a3ff2d46f52059d0d06a32bc93f300dd13d/src/plugins/vis_type_timeseries/server/lib/vis_data/helpers/overwrite.js and include a trivial usage of is-glob then build the artifact. You could see is-glob is built in.

import set from 'set-value';
import isGlob from 'is-glob';

console.log(isGlob('abc'));  // false
console.log(isGlob('*.js'));  // true

/**
 * Set path in obj. Behaves like lodash `set`
 * @param obj The object to mutate
 * @param path The path of the sub-property to set
 * @param val The value to set the sub-property to
 */
export function overwrite(obj, path, val) {
  set(obj, path, undefined);
  set(obj, path, val);
}

Try another example isarray from lodash. This isarray is as well only shown in yarn.lock and @osd//optimizer depends on it. Also add some trivial usage in the same file:

import isarray from 'isarray';

console.log(isarray([])); // true
console.log(isarray('not an array')); // false

But I see isarray is built in artifact both in main and node 18 branch.

With the above two examples, I suspect the issue is from set-value package itself. Currently our solution is to add it as a dependency in package.json to force it to be built in the artifact. This might need further research on it after node 18 is bumped.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants