Skip to content

fix function signatures for osmAuth - #147

Closed
k-yle wants to merge 1 commit into
osmlab:mainfrom
k-yle:kh/not-class
Closed

fix function signatures for osmAuth#147
k-yle wants to merge 1 commit into
osmlab:mainfrom
k-yle:kh/not-class

Conversation

@k-yle

@k-yle k-yle commented Mar 3, 2026

Copy link
Copy Markdown
Member

Follow up from openstreetmap/iD#11661 (comment)

This reverts #125, since that PR broke IDE-autocomplete and type-safety for anyone who uses x = osmAuth() rather than x = new osmAuth().

Declaring osmAuth as an ES6 class is wrong, since it's just a function. Therefore it's valid to instantiate the class without the new operator. The example in the README also does not use new.

This is not an exact revert of #125, instead it settles on a compromise that should work for everyone. Therefore, this is not a breaking change.


Tested by creating a new file in the repo called test.ts with the following content:

import { osmAuth } from 'osm-auth';

const a = osmAuth({
  client_id: '',
  scope: '',
  redirect_uri: '',
  // @ts-expect-error -- typo should be detected
  invalid: 1,
});
a.authenticate(console.log);
// @ts-expect-error -- typo should be detected
a.invalid();

const b = new osmAuth({
  client_id: '',
  scope: '',
  redirect_uri: '',
  // @ts-expect-error -- typo should be detected
  invalid: 1,
});
b.authenticate(console.log);
// @ts-expect-error -- typo should be detected
b.invalid();

If the file compiles with no errors, that proves that this PR works for both cases.

@bhousel

bhousel commented Mar 3, 2026

Copy link
Copy Markdown
Member

We should probably just rewrite this thing to be an actual ES6 class.

@k-yle

k-yle commented Mar 4, 2026

Copy link
Copy Markdown
Member Author

We should probably just rewrite this thing to be an actual ES6 class.

Would you accept a PR that does this? the diff would be massive (adding this. as a prefix to many functions/variables), so it might be hard to review.

maybe the best short-term option is just to update the README, to suggest that people use new? If you use new, the existing definitions works well

@bhousel

bhousel commented Mar 4, 2026

Copy link
Copy Markdown
Member

Yeah for now let's just update the docs to encourage new, that seems like the simplest thing.

@k-yle

k-yle commented Mar 4, 2026

Copy link
Copy Markdown
Member Author

cool, then I'll close this and we can follow up on the docs in #148

@k-yle k-yle closed this Mar 4, 2026
@k-yle
k-yle deleted the kh/not-class branch March 4, 2026 14:02
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