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

Replace xml2js with fast-xml-parser #273

Merged
merged 2 commits into from
Aug 19, 2024
Merged

Replace xml2js with fast-xml-parser #273

merged 2 commits into from
Aug 19, 2024

Conversation

janybravo
Copy link

@janybravo janybravo commented Aug 2, 2024

To remove dependency/peerDependency of xml2js that causes problems with VITE. Backwards compatibility with xml2js is preserved by using XmlParserOptions export with API of fast-xml-parse.

@janybravo janybravo force-pushed the update-dependencies2 branch from 976db90 to 766e406 Compare August 2, 2024 12:41
@janybravo janybravo force-pushed the update-dependencies2 branch 2 times, most recently from b4f4c5b to 48aaf64 Compare August 2, 2024 13:50
@zcernigoj
Copy link
Collaborator

zcernigoj commented Aug 2, 2024

As far as I tested, everything works as before. But I would also ask someone else to additionally check.

The things I found out that also don't work on master (published version):

1: Time dimension is in a different place in SH WMS GetCapabilities response - it's not in every layer, but under the parent Layer element in the XML.
So, the WMSLayer.findDatesUTC fails, because it assumes that the time dimension is in every layer (https://github.com/sentinel-hub/sentinelhub-js/blob/master/src/layer/WmsLayer.ts#L69). Probably best to solve in a different MR.

2: WMTSLayer doesn't have the findDatesUTC() implemented, so we "can't test" if the dates are parsed correctly from the WMTS GetCapabilities response.
Getting dates from WMTS GetCapabilities response was implemented in EO Browser directly in the past, and now no WMTS datasources are used, so not needed from our side. Might be useful for others though. Can be solved in a separate MR.

3: EDIT AFTER this commit

The build process now works, but the npm i still changes the package-lock.json file - it removes the part below

      "dependencies": {
        "@rollup/rollup-linux-x64-gnu": "*"
      },
old error

The build process fails for me (on linux) with the error below and the mentioned solution (removing node_modules and package-lock.json) works.
The problem is that after installing dependencies again, the package-lock.json is totally different from before. (happens for both node 18 and 20)

.../sentinelhub-js(update-dependencies2)$ npm run build

> @sentinel-hub/[email protected] build
> rollup -c

.../sentinelhub-js/node_modules/rollup/dist/native.js:59
                throw new Error(
                ^

Error: Cannot find module @rollup/rollup-linux-x64-gnu. npm has a bug related to optional dependencies (https://github.com/npm/cli/issues/4828). Please try `npm i` again after removing both package-lock.json and node_modules directory.
    at requireWithFriendlyError (.../sentinelhub-js/node_modules/rollup/dist/native.js:59:9)
    at Object.<anonymous> (.../sentinelhub-js/node_modules/rollup/dist/native.js:68:76)
    ... 3 lines matching cause stack trace ...
    at Module._load (node:internal/modules/cjs/loader:1013:12)
    at Module.require (node:internal/modules/cjs/loader:1225:19)
    at require (node:internal/modules/helpers:177:18)
    at Object.<anonymous> (.../sentinelhub-js/node_modules/rollup/dist/shared/parseAst.js:12:19)
    at Module._compile (node:internal/modules/cjs/loader:1356:14) {
  [cause]: Error: Cannot find module '@rollup/rollup-linux-x64-gnu'
  Require stack:
  - .../sentinelhub-js/node_modules/rollup/dist/native.js
  - .../sentinelhub-js/node_modules/rollup/dist/shared/parseAst.js
  - .../sentinelhub-js/node_modules/rollup/dist/shared/rollup.js
  - .../sentinelhub-js/node_modules/rollup/dist/bin/rollup
      at Module._resolveFilename (node:internal/modules/cjs/loader:1134:15)
      at Module._load (node:internal/modules/cjs/loader:975:27)
      at Module.require (node:internal/modules/cjs/loader:1225:19)
      at require (node:internal/modules/helpers:177:18)
      at requireWithFriendlyError (.../sentinelhub-js/node_modules/rollup/dist/native.js:41:10)
      at Object.<anonymous> (.../sentinelhub-js/node_modules/rollup/dist/native.js:68:76)
      at Module._compile (node:internal/modules/cjs/loader:1356:14)
      at Module._extensions..js (node:internal/modules/cjs/loader:1414:10)
      at Module.load (node:internal/modules/cjs/loader:1197:32)
      at Module._load (node:internal/modules/cjs/loader:1013:12) {
    code: 'MODULE_NOT_FOUND',
    requireStack: [
      '.../sentinelhub-js/node_modules/rollup/dist/native.js',
      '.../sentinelhub-js/node_modules/rollup/dist/shared/parseAst.js',
      '.../sentinelhub-js/node_modules/rollup/dist/shared/rollup.js',
      '.../sentinelhub-js/node_modules/rollup/dist/bin/rollup'
    ]
  }
}

Node.js v18.19.0

@janybravo janybravo force-pushed the update-dependencies2 branch from 6dfb29a to 6c3c980 Compare August 19, 2024 09:28
@janybravo janybravo merged commit 7ace13b into master Aug 19, 2024
2 checks passed
@janybravo janybravo deleted the update-dependencies2 branch August 19, 2024 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants