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

Node 18 #3333

Closed
wants to merge 1 commit into from
Closed

Node 18 #3333

wants to merge 1 commit into from

Conversation

jpan-box
Copy link
Contributor

@jpan-box jpan-box commented May 24, 2023

To build storybook and styleguidist:

  • nvm install 18.15.0
  • nvm use 18.15.0

@@ -73,7 +72,7 @@
"release:latest": "DIST=latest BRANCH=master ./scripts/release.sh",
"release:next": "DIST=next BRANCH=next ./scripts/release.sh",
"release:cdn": "yarn setup; node ./scripts/prod.js",
"setup": "yarn install --frozen-lockfile; npm-run-all clean build:i18n",
"setup": "yarn install --frozen-lockfile --ignore-optional; npm-run-all clean build:i18n",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fsevents is an optional dependency for many packages, and requires python2

@@ -297,7 +296,6 @@
"sass": "1.34.1",
"sass-lint": "^1.13.1",
"sass-loader": "^8.0.2",
"sass-variable-parser": "^1.2.2",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

sass-variable-parser has a dependency on node-gyp, which requires python2. we should replace that with a manual process to update variables, since that is done so rarely

@@ -54,7 +54,6 @@
"build:prod:npm": "BABEL_ENV=production OUTPUT=dist LANGUAGE=en-US REACT=true yarn build:prod:dist",
"build:prod:storybook": "LANGUAGE=en-US REACT=true BROWSERSLIST_ENV=production BABEL_ENV=development NODE_ENV=development build-storybook -c .storybook -o styleguide/storybook",
"build:sync": "LANGUAGE=en-US BABEL_ENV=development NODE_ENV=development RSYNC=true webpack --config scripts/webpack.config.js --mode development",
"build:variables": "cat `find src/styles/constants -name '*.scss'` > src/styles/variables.scss | node ./scripts/build-style-vars.js src/styles/variables.scss && rm src/styles/variables.scss",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removing usage of build-style-vars which uses sass-variable-parser

"start": "npm-run-all setup start:examples",
"start:dev": "yarn setup; LANGUAGE=en-US BABEL_ENV=development NODE_ENV=development node --max_old_space_size=8192 node_modules/webpack-dev-server/bin/webpack-dev-server.js --config scripts/webpack.config.js --mode development",
"start:examples": "EXAMPLES=true LANGUAGE=en-US REACT=true BABEL_ENV=development NODE_ENV=development node --max_old_space_size=8192 node_modules/react-styleguidist/bin/styleguidist.js server --config scripts/styleguide.config.js --mode development",
"start:examples": "EXAMPLES=true LANGUAGE=en-US REACT=true BABEL_ENV=development NODE_ENV=development node --openssl-legacy-provider --max_old_space_size=8192 node_modules/react-styleguidist/bin/styleguidist.js server --config scripts/styleguide.config.js --mode development",
Copy link
Contributor Author

@jpan-box jpan-box May 24, 2023

Choose a reason for hiding this comment

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

node 17 introduced some ssl security fixes: https://stackoverflow.com/questions/69692842/error-message-error0308010cdigital-envelope-routinesunsupported

Must be fixed prior to release.

@jpan-box jpan-box changed the title first try Node 18 May 25, 2023
"start:examples:legacy": "BROWSERSLIST_ENV=production yarn start:examples",
"start:npm": "yarn setup; yarn build:dev:es",
"start:storybook": "LANGUAGE=en-US REACT=true BABEL_ENV=development NODE_ENV=development start-storybook -p 6061",
"start:storybook": "LANGUAGE=en-US REACT=true BABEL_ENV=development NODE_ENV=development NODE_OPTIONS=--openssl-legacy-provider start-storybook -p 6061",
Copy link
Contributor Author

@jpan-box jpan-box May 25, 2023

Choose a reason for hiding this comment

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

Same SSL issue as styleguidist.

Storybook 5.3.6 does not seem to require a specific version of node: https://github.com/storybookjs/storybook/blob/94aca9f154a0ff6862bd8407d6769cbb6c90b1d9/package.json#L234

@@ -739,7 +739,7 @@ describe('features/content-explorer/content-explorer/ContentExplorer', () => {
expect(result).toStrictEqual({});
});

test('should call selectAll when handleSelectAllClick and checkbox is not selected', () => {
test.skip('should call selectAll when handleSelectAllClick and checkbox is not selected', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

skipped due to error

BABEL_ENV=test NODE_ENV=test yarn jest -c scripts/jest/jest.config.js src/features/content-explorer/content-explorer/__tests__/ContentExplorer.test.js -t "on"
yarn run v1.22.17
$ /Users/jpan/Development/Box/box-ui-elements/node_modules/.bin/jest -c scripts/jest/jest.config.js src/features/content-explorer/content-explorer/__tests__/ContentExplorer.test.js -t on
Browserslist: caniuse-lite is outdated. Please run the following command: `yarn upgrade`

 RUNS  src/features/content-explorer/content-explorer/__tests__/ContentExplorer.test.js
/Users/jpan/Development/Box/box-ui-elements/src/features/content-explorer/content-explorer/ContentExplorer.js:502
      var selectedItemsIds = Object.keys(selectedItems);
                                    ^

@@ -667,7 +667,7 @@ describe('features/unified-share-modal/UnifiedShareForm', () => {
},
);

test.each`
test.each.skip`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Skipped due to error

$ BABEL_ENV=test NODE_ENV=test yarn jest -c scripts/jest/jest.config.js src/features/unified-share-modal/__tests__/UnifiedShareForm.test.js -t "should fetch justification reasons when collab restrictions change, justification is allowed and restricted collabs are present"
yarn run v1.22.17
$ /Users/jpan/Development/Box/box-ui-elements/node_modules/.bin/jest -c scripts/jest/jest.config.js src/features/unified-share-modal/__tests__/UnifiedShareForm.test.js -t 'should fetch justification reasons when collab restrictions change, justification is allowed and restricted collabs are present'

 RUNS  src/features/unified-share-modal/__tests__/UnifiedShareForm.test.js
/Users/jpan/Development/Box/box-ui-elements/src/features/unified-share-modal/UnifiedShareForm.js:119
          justificationReasons: options.map(function (_ref2) {
                                        ^

TypeError: Cannot read properties of undefined (reading 'map')
    at map (/Users/jpan/Development/Box/box-ui-elements/src/features/unified-share-modal/UnifiedShareForm.js:120:51)

Node.js v18.15.0
error Command failed with exit code 1.

@@ -120,7 +119,7 @@
}
},
"engines": {
"node": ">=14.0.0 <15.0.0",
"node": ">=14.0.0 <18.16.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

18.16.0 breaks Jest, likely due to nodejs/node#47563

Summary of all failing tests
 FAIL  src/elements/content-preview/__tests__/ContentPreview.test.js (40.869s)
   elements/content-preview/ContentPreview  constructor()  should emit when js loaded

    TypeError: Cannot redefine property: performance

      29 |             this.updateContentInsightsOptions = jest.fn();
      30 |         };
    > 31 |         global.performance = {
         |         ^
      32 |             now: jest.fn().mockReturnValue(PERFORMANCE_TIME),
      33 |         };
      34 |     });

      at Object.<anonymous> (src/elements/content-preview/__tests__/ContentPreview.test.js:31:9)
      at processTicksAndRejections (node:internal/process/task_queues:95:5)

@greg-in-a-box
Copy link
Contributor

closing in favor for #3347

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

Successfully merging this pull request may close these issues.

None yet

2 participants