Webpack5 comments #7450

zoran995 · 2025-01-29T15:12:27Z

What this PR does

Updates the versions of webpack plugins and cleanup no longer used ones.
Resolves the Ion default token message issue. Depends on move default token message to Ion property cesium#88 being merge and release first

Test me

Locally:

sync terriamap-terriajs dependencies
install dependencies
run the project
check that everything works the same (type checking is still working, assimpjs works correctly (copy-plugin-webpack))

Checklist

There are unit tests to verify my changes are correct or unit tests aren't applicable (if so, write quick reason why unit tests don't exist)
I've updated relevant documentation in doc/.
I've updated CHANGES.md with what I changed.
I've provided instructions in the PR description on how to test this PR.

…er without a style configured

The eslint-recommended configuration is a subset of recommended, so switch to the recommended configuration and remove the rules that are already part of the recommended configuration. There are a few rules that result in numerous errors, so disable them for now.

Remove the dark-matter image now that it is no longer referenced in the tests. This should have been a part of TerriaJS#7314.

Traverse the file system once, and keep that result in a variable.

Replace the in-file var declarations with let/const. Leave the imports as-is to avoid conflicts with the CJS to ESM upgrade.

I got this warning when running some command: npm warn deprecated inflight@1.0.6: This module is not supported, and leaks memory. Do not use it. Check out lru-cache if you want a good and tested way to coalesce async requests by a key value, which is much more comprehensive and powerful. and inflight is a dependency of glob, which glob-all depends on. So replace glob-all with fast-glob that both seems fast and is already used by some other dependencies.

This was previously used for loading polyfills from polyfill.io then we switched to core-js.

ESLint: use recommended configuration

This saves the compiler the effort of having to infer the types during compilation, which shortens compilation times. This commit is just the code fixes, but there is also a typescript-eslint rule for this: https://typescript-eslint.io/rules/explicit-module-boundary-types/ The annotations were generated from the fixes in the TypeScript compiler with: ts-fix -t ./tsconfig-node.json -e 9008 --write --ignoreGitStatus --file lib/**/*.ts

These functions do not await anything, so remove the async from them. This will become an error when switching to the typed lint rules in typescript-eslint.

To avoid breaking change to terriamap. We'll make this change when upgrading to webpack5.

CJS to ESM

Move release guide to separate file

…flet

zoran995 · 2025-02-02T16:33:21Z

lib/ReactViews/ExplorerWindow/Tabs/MyDataTab/my-data-tab.scss

@@ -136,9 +136,6 @@

 .btn--back-to-my-data {
  composes: btn from "../../../../Sass/common/_buttons.scss";
-  // TODO: fix?
-  // composes: btn-small from "../../../../Sass/common/_buttons.scss";
-  // composes: btn--left from "../../../../Sass/common/_buttons.scss";


btn--left was an icon definition so it is safe to remove
btn-small wouldn't make any point here as we have all its styles defined/overriden here or in styled-component

zoran995 · 2025-02-02T16:33:49Z

lib/Sass/common/_buttons.scss

@@ -36,15 +36,6 @@ $btn-setting-size: 35px;
  margin: 0;
 }

-// TODO: fix?
-// .btn-small {


This is not used anywhere anymore

Upgrade protomaps leaflet and remove mvt imagery provider

Upgrade upload-artifact action to v4

pjonsson · 2025-02-06T22:21:11Z

package.json

  "description": "Geospatial data visualization platform.",
  "license": "Apache-2.0",
  "engines": {
    "node": ">= 16.0.0"
  },
-  "sideEffects": false,


This is apparently important for tree shaking. There is a granular version of sideEffects where you provide paths, and I think that would make sense since there are so few imports with side effects in the tree currently. Long-term, I'm guessing you want to get rid of side effecting imports.

It is not always easy to properly setup the list of files, and I don't want to block the webpack upgrade with that

True, it's already been sitting for 2 months, would be nice to get these big PRs into main.

Yeah, we should remove this setting. I was mostly testing to see if it broke anything - it doesn't seem to. I think we can revisit this later.

pjonsson · 2025-02-06T22:27:10Z

Your change to that other package looks straightforward and should be a matter of just merging, and then getting this moving. Do you know what is blocking the progress with getting this merged?

The diff view is no longer useful for reviewing after merging main, but besides me wishing for not completely eliminating the sideEffects directive I think what was here before merging main into the PR looked good.

lib/ReactViews/Notification/NotificationWindow.jsx

zoran995 · 2025-02-07T21:51:18Z

@na9da I had to restore worker-loader as webpack native web worker support tends to get pretty complex, I will keep looking for a fix but it might be good to keep worker-loader and handle that as a follow-up
p. S. Important note: webpack worker support behaves differently in development and production builds. That's why it works on CI but fails locally. I have run into this issue in the past but still don't have a solution that always works

pjonsson · 2025-02-08T12:55:52Z

lib/ReactViews/Custom/Chart/ChartPanelDownloadButton.tsx

-    new URL("./downloadHrefWorker.js", import.meta.url)
-  );
+  const worker = await import(
+    //@ts-expect-error: aaa


I assume you know the reason you had to put this ts-expect-error in, put that instead of "aaa".

And style wise, no other ts-expect-error in terriajs has a colon after it, and most of them have a space between the // and @ts-expect-error.

pjonsson · 2025-02-08T12:59:25Z

p. S. Important note: it behaves differently in development and production builds with native support. That's why it works on CI but fails locally.

I don't know what part of the code this concerns, but it seems worth to put a comment in the code at that place so it doesn't get missed/lost the next time that thing is upgraded.

zoran995 · 2025-02-10T15:32:10Z

I have cherry-picked and merged all changes from here into #7351, so it is easier and faster to review
Closing this

tephenavies and others added 30 commits February 26, 2023 15:30

Update protomaps to support fetching from pmtiles archive

d92da0e

Replace MapboxVectorTileImageryProvider for region mapping

40d6f99

Merge remote-tracking branch 'origin/main' into upgrade-protomaps

224fa25

Upgrade to protomaps-leaflet

27ef37f

support custom URL parameters in a GetLegendGraphic request for a lay…

36e6604

…er without a style configured

Update contact for code of conduct

cee4021

Merge branch 'main' into issue/7198

540ae60

Merge branch 'main' into issue/7198

aee5b4d

Remove leftover image

6dba3da

Remove the dark-matter image now that it is no longer referenced in the tests. This should have been a part of TerriaJS#7314.

explicitly set prettier tab width

54a8110

run prettier

9ca5aff

move release guide to separate file

10a57b9

webpack.config: glob once

3bb97a2

Traverse the file system once, and keep that result in a variable.

webpack.config: replace var with let/const

90abbd6

Replace the in-file var declarations with let/const. Leave the imports as-is to avoid conflicts with the CJS to ESM upgrade.

Simplify proj4 conversion.

24af89d

Remove stray file.

d5d9fb3

Update CODE_OF_CONDUCT.md

c2c0812

Rename import name.

54f416b

Use plain proj strings as parameter.

e3f0c4b

Switch CI to main.

70aaa4e

Remove empty polyfill function.

fcf968c

This was previously used for loading polyfills from polyfill.io then we switched to core-js.

Merge pull request TerriaJS#7323 from pjonsson/no-unused-vars-fixes

a7fa8b3

ESLint: use recommended configuration

Merge branch 'main' into cjs-to-esm

c646896

test: remove async from functions

79ad698

These functions do not await anything, so remove the async from them. This will become an error when switching to the typed lint rules in typescript-eslint.

Revert polyfill change.

e856caf

To avoid breaking change to terriamap. We'll make this change when upgrading to webpack5.

Merge pull request TerriaJS#7311 from TerriaJS/cjs-to-esm

619ec17

CJS to ESM

Merge pull request TerriaJS#7345 from zoran995/release-guide

35542b6

Move release guide to separate file

zoran995 and others added 6 commits January 29, 2025 20:09

remove istanbul-instrumenter-loader as unused dependency

ce8e5c2

Merge remote-tracking branch 'origin/main' into upgrade-protomaps-lea…

3235140

…flet

Small fixes

679f5e5

pass correct dev mode param to webpack config

0d878bc

remove unused scss classes

647484a

use already resolve cesiumDir path

2766b1b

zoran995 commented Feb 2, 2025

View reviewed changes

nf-s and others added 8 commits February 4, 2025 11:40

Merge pull request TerriaJS#7144 from TerriaJS/upgrade-protomaps-leaflet

6033137

Upgrade protomaps leaflet and remove mvt imagery provider

Merge pull request TerriaJS#7445 from TerriaJS/upgrade-artifact-action

1413f62

Upgrade upload-artifact action to v4

remove sideeffects: false from package.json

ce3f633

update yarn.lock

e33f528

Merge remote-tracking branch 'upstream/main' into webpack5-comments

157c3cb

fix protomaps import warning

e32e129

fix protomap build issue

df3bd46

convert polyfill.js to esm

86ad240

pjonsson reviewed Feb 6, 2025

View reviewed changes

pjonsson reviewed Feb 7, 2025

View reviewed changes

lib/ReactViews/Notification/NotificationWindow.jsx Show resolved Hide resolved

resolve linting issues

c7aa95c

zoran995 force-pushed the webpack5-comments branch from f650e06 to c7aa95c Compare February 7, 2025 10:40

zoran995 requested review from na9da and ljowen February 7, 2025 21:49

pjonsson reviewed Feb 8, 2025

View reviewed changes

feat: restore worker-loader

ffd19ab

zoran995 force-pushed the webpack5-comments branch from ca39041 to ffd19ab Compare February 8, 2025 17:56

zoran995 closed this Feb 10, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Webpack5 comments #7450

Webpack5 comments #7450

zoran995 commented Jan 29, 2025

zoran995 Feb 2, 2025

zoran995 Feb 2, 2025

pjonsson Feb 6, 2025

zoran995 Feb 6, 2025

pjonsson Feb 7, 2025

na9da Feb 9, 2025

pjonsson commented Feb 6, 2025

zoran995 commented Feb 7, 2025 •

edited

Loading

pjonsson Feb 8, 2025

pjonsson commented Feb 8, 2025

zoran995 commented Feb 10, 2025

Webpack5 comments #7450

Webpack5 comments #7450

Conversation

zoran995 commented Jan 29, 2025

What this PR does

Test me

Checklist

zoran995 Feb 2, 2025

Choose a reason for hiding this comment

zoran995 Feb 2, 2025

Choose a reason for hiding this comment

pjonsson Feb 6, 2025

Choose a reason for hiding this comment

zoran995 Feb 6, 2025

Choose a reason for hiding this comment

pjonsson Feb 7, 2025

Choose a reason for hiding this comment

na9da Feb 9, 2025

Choose a reason for hiding this comment

pjonsson commented Feb 6, 2025

zoran995 commented Feb 7, 2025 • edited Loading

pjonsson Feb 8, 2025

Choose a reason for hiding this comment

pjonsson commented Feb 8, 2025

zoran995 commented Feb 10, 2025

zoran995 commented Feb 7, 2025 •

edited

Loading