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

Update deps, upgrade to use Vite 3, fix broken API docs #145

Merged
merged 7 commits into from
Aug 11, 2022

Conversation

smellyshovel
Copy link
Collaborator

@smellyshovel smellyshovel commented Aug 5, 2022

Update the dependencies. Use Vite 3. Rollback to TypeDoc 0.22 (resolves #135). Provide CommonJS build (just in case someone needs it).

@smellyshovel smellyshovel added the semver:patch For non-breaking PR's that don't introduce new features label Aug 5, 2022
@smellyshovel smellyshovel added the dependencies Pull requests that update a dependency file label Aug 5, 2022
@smellyshovel smellyshovel changed the title Update deps and fix #135 Update deps, upgrade to use Vite 3, fix broken API docs Aug 5, 2022
@smellyshovel
Copy link
Collaborator Author

@wipfli ready to be merged.

const parsedFileName = path.match(/\/(\d+)\s([^/]+)\./);
const index = parseInt(parsedFileName[1]);
const name = parsedFileName[2];
export const examples = Object.entries(import.meta.glob<ModuleNamespace>("./examples/**.svelte", { eager: true })).map(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

globEager has become deprecated in Vite 3. Hence the upgrade.

@@ -14,7 +14,7 @@
"files": [
"dist"
],
"module": "./dist/maplibre-gl-directions.es.js",
"module": "./dist/maplibre-gl-directions.js",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Vite 3 generated module.js for ESM and module.cjs for CommonJS. In Vite 2 it was module.es.js and module.js respectively.

"svelte-preprocess": "^4.10.7",
"tailwindcss": "^3.1.8",
"tslib": "^2.4.0",
"typedoc": "^0.22.18",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Too many backwards-incompatible updates between 0.22 and 0.23. Rollback to use 0.22 is an easier fix.

@@ -13,7 +13,7 @@ export default defineConfig({

lib: {
entry: "src/main.ts",
formats: ["es"],
formats: ["es", "cjs"],
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just in case someone decides to use in a Webpack-based app (AFAIK, Webpack still doesn't support ES Modules up to this day 😕).

@wipfli
Copy link
Member

wipfli commented Aug 7, 2022

Thanks for this pull request, @smellyshovel. I get a build error:

➜  maplibre-gl-directions git:(smellyshovel/main) npm i      

> @maplibre/maplibre-gl-directions@0.2.0 prepare
> husky install

husky - Git hooks installed

changed 79 packages, and audited 398 packages in 14s

86 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
npm notice 
npm notice New minor version of npm available! 8.11.0 -> 8.16.0
npm notice Changelog: https://github.com/npm/cli/releases/tag/v8.16.0
npm notice Run npm install -g npm@8.16.0 to update!
npm notice 
➜  maplibre-gl-directions git:(smellyshovel/main) npm run env:prep

> @maplibre/maplibre-gl-directions@0.2.0 env:prep
> npm run build:lib && npm link && npm link @maplibre/maplibre-gl-directions


> @maplibre/maplibre-gl-directions@0.2.0 build:lib
> npm run check:lib && tsc --project ./tsconfig.lib.json && vite build --config vite.lib.config.ts


> @maplibre/maplibre-gl-directions@0.2.0 check:lib
> svelte-check --tsconfig ./tsconfig.lib.json --workspace src


====================================
Loading svelte-check in workspace: /home/wipfli/maps/maplibre-gl-directions/src
Getting Svelte diagnostics...

/home/wipfli/maps/maplibre-gl-directions/src/directions/events.ts:17:42
Hint: Parameter 'listener' implicitly has an 'any' type, but a better type may be inferred from usage. 

    this.listeners[event.type]?.forEach((listener) => listener(event));                                                           
    this.oneTimeListeners[event.type]?.forEach((listener) => {                                                                    


/home/wipfli/maps/maplibre-gl-directions/src/directions/events.ts:18:49
Hint: Parameter 'listener' implicitly has an 'any' type, but a better type may be inferred from usage. 
    this.listeners[event.type]?.forEach((listener) => listener(event));
    this.oneTimeListeners[event.type]?.forEach((listener) => {                                                                    
      listener(event);                                                                                                            


/home/wipfli/maps/maplibre-gl-directions/src/directions/events.ts:127:3
Hint: Member 'type' implicitly has an 'any' type, but a better type may be inferred from usage. 

  type;                                                                                                                           
  target!: Map;


/home/wipfli/maps/maplibre-gl-directions/src/directions/events.ts:149:3
Hint: Member 'type' implicitly has an 'any' type, but a better type may be inferred from usage. 

  type;
  target!: Map;


/home/wipfli/maps/maplibre-gl-directions/src/directions/main.ts:121:11
Hint: Variable 'timer' implicitly has an 'any' type, but a better type may be inferred from usage. 

      let timer;
      if (this.configuration.requestTimeout !== null) {


/home/wipfli/maps/maplibre-gl-directions/src/directions/main.ts:390:51
Hint: 'which' is deprecated. 
    if (e.type === "touchstart" && e.originalEvent.touches.length !== 1) return;
    if (e.type === "mousedown" && e.originalEvent.which !== 1) return;



/home/wipfli/maps/maplibre-gl-directions/src/directions/main.ts:514:51
Hint: 'which' is deprecated. 
    if (e.type === "touchmove" && e.originalEvent.touches.length !== 1) return e.originalEvent.preventDefault();
    if (e.type === "mousemove" && e.originalEvent.which !== 1) return;



/home/wipfli/maps/maplibre-gl-directions/src/directions/main.ts:551:49
Hint: 'which' is deprecated. 

    if (e.type === "mouseup" && e.originalEvent.which !== 1) return;



====================================
svelte-check found 0 errors, 0 warnings, and 8 hints
src/controls/loading-indicator/main.ts:2:46 - error TS2307: Cannot find module './LoadingIndicatorControl.svelte' or its corresponding type declarations.

2 import LoadingIndicatorControlComponent from "./LoadingIndicatorControl.svelte";
                                               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Found 1 error in src/controls/loading-indicator/main.ts:2

@wipfli
Copy link
Member

wipfli commented Aug 7, 2022

If I use npm ci instead of npm i, like it is done in the GitHub Action Workflow, the env:prep step passes...

@smellyshovel
Copy link
Collaborator Author

smellyshovel commented Aug 8, 2022

@wipfli

If I use npm ci instead of npm i, like it is done in the GitHub Action Workflow, the env:prep step passes...

Yeah, I also installed the deps with npm ci. I'm not sure why it's so. But I guess it's OK after one time of npm ci? Meaning that the next time you install the deps it works with npm i, no?

@wipfli
Copy link
Member

wipfli commented Aug 8, 2022

I guess calling npm ci before npm i can work as a workaround, but starting from a clean installation, the instructions in the contributing guide should work...

@smellyshovel
Copy link
Collaborator Author

@wipfli updated the CONTRIBUTING's "troubleshooting" section. Since the build passes both here and for me and for you, I don't think anything should be done to solve the initial issue (that turns out not to exist at all). I think it's some caching (or whatever) related problem of NPM.

Copy link
Member

@wipfli wipfli left a comment

Choose a reason for hiding this comment

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

Thanks @smellyshovel! I run this pull request in a clean GitHub codespaces environment and everything worked. So probably this was just my local environment. Sorry for the trouble...

@wipfli wipfli merged commit c1b2273 into maplibre:main Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file semver:patch For non-breaking PR's that don't introduce new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Broken API docs
2 participants