search

johannschopplich / kirbyup

🆙 Official bundler for Kirby Panel plugins
https://kirbyup.getkirby.com
MIT License
51 stars 3 forks source link

fix: set section flag independent from usedDef check #28

Closed jonaskuske closed 2 years ago

jonaskuske commented 2 years ago

The .$_isSection flag (that's used to detect section components and re-apply the section mixin on HMR reload) was only added if the component definition held by the HMR runtime was stale. So if Kirby were to update its plugin code to mutate the component definition of section components instead of replacing it, the HMR plugin would break because the map[id].options !== usedDefinition condition wouldn't trigger anymore and the .$_isSection flag would not be set. Now, the flag is added independently from that condition, which is more sound.

Also, added the /* injected by kirbyup */ banner again, which went missing during some refactor. (which is why the "end" comment looked lost and didn't make sense)

jonaskuske commented 2 years ago

After this PR I can try releasing an updated beta to npm 🤞

johannschopplich commented 2 years ago

LGTM. Let's merge it and then feel free to release a beta. Have fun! 🙌

jonaskuske commented 2 years ago

Alright, I think 2.1.0 can be released.

Buuuuut I just found out that I've actually released it already, sorry! :( Didn't know that the release/bumpp command does not automatically adjust the npm tag, so now the release is called 2.1.0-beta.1, but I've actually released it under the latest tag on npm so it's installed when you just run npm i kirbyup 😅

I think renaming the command to release:prod and adding a separate script release:dev that does bumpp --tag dev would be helpful to avoid that in the future.

jonaskuske commented 2 years ago

npm publish is run from the GitHub Action and never changes the npm tag, so I guess that bug's always been there? https://github.com/johannschopplich/kirbyup/blob/main/.github/workflows/release.yml#L30

johannschopplich commented 2 years ago

Oh shoot, good catch! Didn't know about this caveat. Makes sense now tho. 🙈 Yep, it has always been that way.

Your other PR will fix that, thanks!