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

[webpack-build-scripts] feat: migrate to swc #6453

Merged
merged 19 commits into from
Oct 10, 2023
Merged

Conversation

adidahiya
Copy link
Contributor

@adidahiya adidahiya commented Oct 9, 2023

Changes proposed in this pull request:

  • Migrate from ts-loader to swc-loader
  • Configure SWC for webpack bundling
  • Fix webpack static option to fix HMR
  • Upgrade SWC (patch version)
  • Remove react-refresh-typescript (no longer required)
  • Enable importsNotUsedAsValues: "error" tsc option and annotate all type-only imports & exports in TypeScript
  • Update Karma coverageExcludes to skip checking coverage on barrel files (index.ts in various locations) since the emitted .js files now look a little different and there seem to be more lines of code emitted, which Istanbul tries to instrument for coverage
  • Refactor unit tests which relied on Sinon to stub entire ES modules, which seems to be unsupported in the official spec since those modules are "sealed" and non-configurable / non-writable once defined & exported (ts-loader was more lenient about this, it generated writable module exports which Sinon could more easily stub)

Reviewers should focus on:

✅ test suites run all tests successfully

✅ TypeScript errors still fail the webpack production build:

/Users/adi/Developer/repos/oss/blueprint/packages/docs-app  [ ad ] ✈  yarn dist
yarn run v1.22.19
$ cross-env NODE_ENV=production yarn bundle
$ webpack
assets by status 21.2 MiB [cached] 621 assets
Entrypoint docs-app = docs-app.css docs-app.js 11 auxiliary assets
orphan modules 10.9 MiB (javascript) 1.72 MiB (asset) 85.9 KiB (runtime) [orphan] 4029 modules
runtime modules 36.3 KiB 15 modules
built modules 20.4 MiB (javascript) 498 KiB (css/mini-extract) [built]
  javascript modules 15.9 MiB 1650 modules
  css modules 498 KiB
    modules by path ../../node_modules/monaco-editor/esm/vs/editor/ 64.5 KiB 57 modules
    modules by path ../../node_modules/monaco-editor/esm/vs/base/browser/ 30.1 KiB 25 modules
    modules by path ../../node_modules/monaco-editor/esm/vs/platform/ 9.88 KiB 5 modules
    + 9 modules
  json modules 4.52 MiB
    modules by path ./src/ 7.17 KiB 2 modules
    modules by path ../ 4.52 MiB
      ../docs-data/src/generated/docs.json 4.43 MiB [built] [code generated]
      ../icons/icons.json 91.6 KiB [built] [code generated]

ERROR in ./src/examples/datetime2-examples/dateRangePicker3Example.tsx:122:25
TS2322: Type '{ checked: boolean; labl: string; onChange: (event: FormEvent<HTMLElement>) => void; }' is not assignable to type 'IntrinsicAttributes & SwitchProps & { children?: ReactNode; }'.
  Property 'labl' does not exist on type 'IntrinsicAttributes & SwitchProps & { children?: ReactNode; }'. Did you mean 'label'?
    120 |                     <Switch
    121 |                         checked={this.state.reverseMonthAndYearMenus}
  > 122 |                         labl="Reverse month and year menus"
        |                         ^^^^
    123 |                         onChange={this.toggleReverseMonthAndYearMenus}
    124 |                     />
    125 |                 </div>

webpack 5.88.2 compiled with 1 error in 22486 ms
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

✅ And the dev build (with webpack overlay):

image

Screenshot

✅ HMR works (fast):

2023-10-09 13 59 34

@adidahiya adidahiya enabled auto-merge (squash) October 9, 2023 18:01
@adidahiya
Copy link
Contributor Author

Remove extraneous dep

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@adidahiya adidahiya marked this pull request as draft October 9, 2023 18:39
@adidahiya
Copy link
Contributor Author

Looks like there are now some issues with sinon.sub() being called on a namespace import import * as DateFnsLocaleUtils from "../../dateFnsLocaleUtils"

<DatePicker3>
    "before all" hook for "renders .bp5-datepicker"
       1) TypeError: Descriptor for property loadDateFnsLocale is non-configurable and non-writable

Perhaps we should make this loader function configurable so it is easier to test...

@adidahiya
Copy link
Contributor Author

[datetime2] fix test typecheck

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@adidahiya
Copy link
Contributor Author

adidahiya commented Oct 10, 2023

Some more interesting findings related to testing with SWC. It turns out that ts-loader was providing us a convenience which does not line up with the official more "strict" behavior of ES modules, specifically the fact that it allowed modules to export configurable and writable properties which could easily be stubbed or overridden by sinon.stub() or sinon.spy(). This is no longer possible with swc-loader, which leaves us with two options for maintaining our existing tests:

edit: it looks like the seams approach doesn't play nicely with TypeScript / ESM, I get the following errors. So it's better to go with the first option.

ModuleNotFoundError: Module not found: Error: Can't resolve 'module' in '/Users/adahiya/dev/oss/blueprint/node_modules/quibble/lib'
    at /Users/adahiya/dev/oss/blueprint/node_modules/webpack/lib/Compilation.js:2022:28
    at /Users/adahiya/dev/oss/blueprint/node_modules/webpack/lib/NormalModuleFactory.js:817:13
    at eval (eval at create (/Users/adahiya/dev/oss/blueprint/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:10:1)
    at /Users/adahiya/dev/oss/blueprint/node_modules/webpack/lib/NormalModuleFactory.js:275:22
    at eval (eval at create (/Users/adahiya/dev/oss/blueprint/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:9:1)
    at /Users/adahiya/dev/oss/blueprint/node_modules/webpack/lib/NormalModuleFactory.js:448:22
    at /Users/adahiya/dev/oss/blueprint/node_modules/webpack/lib/NormalModuleFactory.js:118:11
    at /Users/adahiya/dev/oss/blueprint/node_modules/webpack/lib/NormalModuleFactory.js:689:25
    at /Users/adahiya/dev/oss/blueprint/node_modules/webpack/lib/NormalModuleFactory.js:893:8
    at /Users/adahiya/dev/oss/blueprint/node_modules/webpack/lib/NormalModuleFactory.js:1013:5
resolve 'module' in '/Users/adahiya/dev/oss/blueprint/node_modules/quibble/lib'
  Parsed request is a module
  using description file: /Users/adahiya/dev/oss/blueprint/node_modules/quibble/package.json (relative path: ./lib)
    Field 'browser' doesn't contain a valid alias configuration
    resolve as module
      /Users/adahiya/dev/oss/blueprint/node_modules/quibble/lib/node_modules doesn't exist or is not a directory
      looking for modules in /Users/adahiya/dev/oss/blueprint/node_modules/quibble/node_modules
        single file module
          using description file: /Users/adahiya/dev/oss/blueprint/node_modules/quibble/package.json (relative path: ./node_modules/module)
            no extension
              Field 'browser' doesn't contain a valid alias configuration
              /Users/adahiya/dev/oss/blueprint/node_modules/quibble/node_modules/module doesn't exist
            .css
              Field 'browser' doesn't contain a valid alias configuration
              /Users/adahiya/dev/oss/blueprint/node_modules/quibble/node_modules/module.css doesn't exist
            .js
              Field 'browser' doesn't contain a valid alias configuration
              /Users/adahiya/dev/oss/blueprint/node_modules/quibble/node_modules/module.js doesn't exist
            .ts
              Field 'browser' doesn't contain a valid alias configuration
              /Users/adahiya/dev/oss/blueprint/node_modules/quibble/node_modules/module.ts doesn't exist
            .tsx
              Field 'browser' doesn't contain a valid alias configuration
              /Users/adahiya/dev/oss/blueprint/node_modules/quibble/node_modules/module.tsx doesn't exist
        /Users/adahiya/dev/oss/blueprint/node_modules/quibble/node_modules/module doesn't exist
      /Users/adahiya/dev/oss/blueprint/node_modules/node_modules doesn't exist or is not a directory
      looking for modules in /Users/adahiya/dev/oss/blueprint/node_modules
        single file module
          using description file: /Users/adahiya/dev/oss/blueprint/package.json (relative path: ./node_modules/module)
            no extension
              Field 'browser' doesn't contain a valid alias configuration
              /Users/adahiya/dev/oss/blueprint/node_modules/module doesn't exist
            .css
              Field 'browser' doesn't contain a valid alias configuration
              /Users/adahiya/dev/oss/blueprint/node_modules/module.css doesn't exist
            .js
              Field 'browser' doesn't contain a valid alias configuration
              /Users/adahiya/dev/oss/blueprint/node_modules/module.js doesn't exist
            .ts
              Field 'browser' doesn't contain a valid alias configuration
              /Users/adahiya/dev/oss/blueprint/node_modules/module.ts doesn't exist
            .tsx
              Field 'browser' doesn't contain a valid alias configuration
              /Users/adahiya/dev/oss/blueprint/node_modules/module.tsx doesn't exist
        /Users/adahiya/dev/oss/blueprint/node_modules/module doesn't exist
      /Users/adahiya/dev/oss/node_modules doesn't exist or is not a directory
      /Users/adahiya/dev/node_modules doesn't exist or is not a directory
      /Users/adahiya/node_modules doesn't exist or is not a directory
      /Users/node_modules doesn't exist or is not a directory
      /node_modules doesn't exist or is not a directory

@adidahiya adidahiya marked this pull request as ready for review October 10, 2023 17:31
@adidahiya
Copy link
Contributor Author

[table] fix DragSelectable tests

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@adidahiya adidahiya merged commit 351030f into develop Oct 10, 2023
11 of 12 checks passed
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

1 participant