BREAKING CHANGE: GAUD-9693 - Remove CommonJS and superagent#69
BREAKING CHANGE: GAUD-9693 - Remove CommonJS and superagent#69
Conversation
| node-version-file: .nvmrc | ||
| cache: 'npm' | ||
| - name: Install dependencies | ||
| run: npm ci --ignore-scripts |
There was a problem hiding this comment.
This is now set in the .npmrc file.
| @@ -1,16 +1,14 @@ | |||
| { | |||
| "name": "siren-parser", | |||
| "version": "9.2.3", | |||
There was a problem hiding this comment.
Merging this is gonna bypass everyone's "ignore v9 for Dependabot" exemptions they've added. So we'll want to move quickly after that.
| Entity.prototype.printEntity = function() { console.log(this) }; | ||
| var parsedEntity = SirenParse('{"class":["foo","bar"]}'); // parsedEntity will have printEntity() |
There was a problem hiding this comment.
I don't know if we want to encourage modifying the prototype like this as I think it'd affect everyone else's usage. Maybe show a way to just create a new class that extends the base class and adds the method?
There was a problem hiding this comment.
So this is currently in the README and was apparently added for UCE. But UCE doesn't actually do this anymore, because it is stuck on v8 like everyone else.
So yeah, we can either just not even mention this, or I can rewrite it as a class extension.
There was a problem hiding this comment.
Looking back at my DMs with Marc about this, it was 100% made of hacks and none of the solutions either of us came up with made us particularly happy... so I will be happy to see it gone 😄
| ```html | ||
| <script type="module" src="siren-parser/global.js"></script> | ||
| <script> | ||
| var parsedEntity = D2L.Hypermedia.Siren.Parse('{"class":["foo","bar"]}'); |
There was a problem hiding this comment.
Ew, are people from BSI using this? Can we kill it?
There was a problem hiding this comment.
Yes, but not a lot and we likely can kill it. I'll take a look.
ryantmer
left a comment
ryantmer
left a comment
There was a problem hiding this comment.
A ceremonial approval since I recognize there's more to come, but this will be a lovely improvement. I had just started looking into somehow getting everybody onto 9 recently (as I realized I'd been merging dependabot updates that nobody was actually benefitting from 🙃), but bumping to 10 makes a lot more sense 👍
| Entity.prototype.printEntity = function() { console.log(this) }; | ||
| var parsedEntity = SirenParse('{"class":["foo","bar"]}'); // parsedEntity will have printEntity() |
There was a problem hiding this comment.
Looking back at my DMs with Marc about this, it was 100% made of hacks and none of the solutions either of us came up with made us particularly happy... so I will be happy to see it gone 😄
4 participants
As discussed with @dlockhart and @bearfriend , instead of updating everyone to
v9(which was causing some errors because of theCommonJSchange and trying to support both it and ESM), we wanted to modernize the repo a bit and do another breaking change where we:CommonJSand only supportESMsuperagentstuff (no longer necessary and is encouraged to be removed everywhere).