search

exercism / codemirror-lang-wren

Wren language support for the CodeMirror code editor.
MIT License
3 stars 4 forks source link

Update to stable CodeMirror 6? #19

Closed JRaspass closed 5 months ago

JRaspass commented 2 years ago

Depending on the old 0.19 versions (and removed libraries like @codemirror/highlight) makes it really hard to use this library in a project using stable CM6 now that it's been released.

https://marijnhaverbeke.nl/blog/codemirror-6.html

joshgoebel commented 2 years ago

@ErikSchierboom I assume if I update this and push a new release that won't harm Exercism because we're (maybe?) pinned to the existing NPM release, or is that a faulty assumption because we're not 1.0 yet?

Actually if we're major pinned and I bump it all the way to 1.0 with the next release, that would be good, yes? Then Exercism could catch up when ready?

Let me know if I'm mistaken here.

joshgoebel commented 2 years ago

makes it really hard to use this library in a project using stable CM6 now that it's been released.

For sure, upgrading to stable would be great.

Do you know what happened to highlight? Did it just get absorbed? I haven't been following CodeMirror - I only built this because it was necessary for full Wren support on Exercism.

JRaspass commented 2 years ago

The deprecated notice says:

As of 0.20.0, this package has been split between @lezer/highlight and @codemirror/language

https://www.npmjs.com/package/@codemirror/highlight

joshgoebel commented 2 years ago

Thanks! I'll look into this after we hear back from Erik. Could you possibly share what project you're using this in? Pretty awesome to see it being used outside of the Exercism ecosystem.

JRaspass commented 2 years ago

Sure, code.golf gained support for Wren today with https://github.com/code-golf/code-golf/commit/f7c0d8a77e637e049e7245a676057e246223f0a0 but no syntax highlighting yet. The frontend is all CM6.

joshgoebel commented 2 years ago

I wonder if there is some best practice on what versions we should pin against to be most useful:

    "@codemirror/autocomplete": "^6.2.0",
    "@codemirror/language": "^6.2.1",
    "codemirror": "^6.0.1",

Is that going to disallow people using 6.0.0?

JRaspass commented 2 years ago

Assuming these changes also work with 6.0.0 (I haven't tested. Then I think ^6 would offer the best compatibility with end users:

image https://semver.npmjs.com

joshgoebel commented 2 years ago

Assuming these changes also work with 6.0.0

I the API is stable I'd assume it should.

joshgoebel commented 2 years ago

After review I'll merge and ship 1.0.0.

SleeplessByte commented 2 years ago

re Exercism: we really were at a beta version before. Bumping it should be okay, because if you bump it, it won't stop Exercism from building as they will need to udpate their log file. If this code language pack works with both the older version and the newer version, there are different ways to accomplish this instead.

re version range: It really depends what you're trying to accomplish.

So for both things, it comes down to the following: this is a language pack. It makes no sense to install this without depending on code mirror. That means that the version requirement is just that: indicatory of what it expects and supports. In this particular case I highly recommend removing any code mirror dependency and instead add those as peer dependencies. This will lead to deduplication no matter what tool the consumer is using and it leads to a warning instead of a double packaged version or error if the version doesn't match.

If you intend to release or recheck this library regularly you could pick ^6.0.0 / 6.x. If you don't care at all but it may break in the future you can do * which means "be installed, don't care which version".

6.x is the same as ^6.0.0 and 6. ^6 is also the same, but not a syntax i recommend.

~ is meant for patch, ^ mean for minor. Setting a lowest x and y that you support is generally not bad.

joshgoebel commented 2 years ago

https://github.com/exercism/codemirror-lang-wren/blob/77cbbbc67ae6aedbbb44c63bf64ce91d71cb33d0/package.json#L30-L34

It sounds like you're suggesting I make all 4 (all of the actual non-dev dependencies) peer, am I understanding correctly?

If you intend to release or recheck this library regularly you could pick ^6.0.0 / 6.x.

Wait, "if I intend to check regularly"... isn't pinning ^6.0.0 then periodic (not regular) option? I'm assuming this is reliable semver, so if it works now it (in theory) shouldn't break until 7. If that's the case and there are no bugs here I'd prefer to not need to touch this again until CodeMirror 7. :-)

ErikSchierboom commented 1 year ago

@ErikSchierboom I assume if I update this and push a new release that won't harm Exercism because we're (maybe?) pinned to the existing NPM release, or is that a faulty assumption because we're not 1.0 yet?

Currently, we're not even pinning to NPM but to the repo:

"@exercism/codemirror-lang-wren": "https://github.com/exercism/codemirror-lang-wren",

See https://github.com/exercism/website/blob/main/package.json#L18

Let me know what you'd like me to pin it to, and then I can update.