Working towards the 1.0.0 release!

ammmze commented 3 years ago

First want to say thanks for all the heavy lifting I've been seeing in the last few months on this project. I see there's a new plugin system coming in the 1.0.0 release. That looks really interesting. Where i'm currently using jsep, I am looking for that "object" plugin. I've got a couple of question...

Do you happen to have any idea when things will be stable enough for a new release?
Is there anything in particular that is outstanding preventing a release that needs addressed?

6utt3rfly commented 3 years ago

@EricSmekens - do you want to publish a release candidate? Or just release as is and publish fixes as needed? We've kept the core of jsep backward-compatible (although #100, which dropped the LogicalExpression type, may be a breaking change for some), so it should be painless to jump to 1.0.0. Maybe we'll just need plugin fixes on their own, if at all?

I'm nearly finished updating expression-eval and confirming that a wide variety of expressions can be parsed and evaluated, but I'm sure there are use-cases that haven't been looked at (including cjs vs esm vs iife), and there may be more error-checking we could add for invalid expressions (i.e. the object plugin adds a binary operator ':' which will parse a: 1 on its own okay, but will then give an error when evaluating). If I get time, I'd also like to go through some of the outstanding issues in jsep to see if some of them can be addressed 🤷🏻‍♀️

LeaVerou commented 3 years ago

I think it would help to have some docs for those migrating too, though that shouldn't be a blocker.

6utt3rfly commented 3 years ago

Are you thinking of a separate file? Or just in the changelog?

LeaVerou commented 3 years ago

Depends on how extensive the guidance is. We could start with just notes in the changelog.

EricSmekens commented 3 years ago

I'll publish 1.0.0 later this week! (Need to find some time where I can walk through it all and prepare myself for it).

ammmze commented 3 years ago

👍🏻 I appreciate everyones hard work on this project. It's awesome to see this project continue to get better! Let me know if there is anything I can help with.

EricSmekens commented 3 years ago

Published a 1.0.0 of the main jsep package. Continued with publishing the packages manually as well, but encountered some permission errors.

Probably due to a @jsep scope is already existing due to this package: https://www.npmjs.com/package/@jsep/popover

Shall we just come up with a alternative on this name, as in @jsep-plugin/arrow to make it scoped packages? Or shall we skip the scoping and release the packages as jsep-plugin-arrow?

6utt3rfly commented 3 years ago

ah too bad about not being able to use @jsep ... I think it would be better to scope if possible, so I like @jsep-plugin/arrow. Should be an easy find/replace?

6utt3rfly commented 3 years ago

@EricSmekens - can the package.json file in the dist/cjs folder be included in the published package? Otherwise, I get this:

Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: node_modules/jsep/dist/cjs/jsep.cjs.js
require() of ES modules is not supported.
require() of node_modules/jsep/dist/cjs/jsep.cjs.js from x.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
Instead rename jsep.cjs.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from /node_modules/jsep/package.json.

Actually - the error indicates that we could rename that build to be jsep.cjs instead of jsep.cjs.js as well?

(Edit: I can create a quick PR if you prefer the .cjs file name extension over the package.json file)

EricSmekens commented 3 years ago

Ah yes, only tested the regular esm import. Great issue for 1.0.1 that we can release on short-term I think. If including package.json will fix this, I think that would be the way to go?

6utt3rfly commented 3 years ago

Yeah the package.json in that folder (which sets the module type for cjs) fixes it, which is why it was there. I haven't seen the cjs extension much, but it sounds like that would work too

6utt3rfly commented 3 years ago

Agree that it's worth a quick 1.0.1 release for that

JiriTrecak commented 3 years ago

Can confirm, this would be a great fix. Currently running into the same issue with the version bumped to 1.0.0. I managed to work around it with different configurations of node packager, but since I am using typescript, it fails to import the types even with fixes - which can't, unfortunately, be fixed any other way than with the improvements to the package.

In other news, good job. You rock, this is amazing piece of work and engineering you have done over the last month <3

EricSmekens commented 3 years ago

It's a bit odd, as I'm preparing a 1.0.1 release, but....

npm pack and npm publish --dry-run does include the dist/cjs/package.json we're talking about. I might perhaps try publish a test-publish with a rc-1 version for 1.0.1, to see something is wrong about the config, or I just make a mistake during first publish... 😣

6utt3rfly commented 3 years ago

Yeah that does seem strange ... and I checked out the github action history and it was in there (although it couldn't publish for some reason?) 🤷🏻‍♀️ . The file made it into the published plugins, too

EricSmekens commented 3 years ago

Yeah, I made the publishes first manually with npm6. (Not realising npm7 has workspaces support, that's great, learning new stuff everyday ;)).

I've also tried this with 1.0.1-alpha and 1.0.1-beta, but I can't get it to work yet.

6utt3rfly commented 3 years ago

I just tried npm install jsep@1.0.1-beta and it looks like it's right edit - same with 1.0.1-alpha

So both seem like they published OK... not sure why the 1.0.0 didn't though

EricSmekens commented 3 years ago

https://github.com/EricSmekens/jsep/actions/workflows/npm-publish.yml

It looks like 1.0.1 fixes this issue by properly including the package.json.

If you could confirm this, that would be great 😀

6utt3rfly commented 3 years ago

Just tried 1.0.1 in a cjs project. Looks good! Thanks!

ammmze commented 3 years ago

I'm pulling this in now to try it out. And it looks like it is functionally there. But it looks like the type declarations didn't make it in the package leaving me with the error TS7016: Could not find a declaration file for module 'jsep'. error when trying to build my package.

❯ tree node_modules/jsep
node_modules/jsep
├── CHANGELOG.md
├── LICENSE
├── README.md
├── dist
│   ├── cjs
│   │   ├── jsep.cjs.js
│   │   ├── jsep.cjs.js.map
│   │   ├── jsep.cjs.min.js
│   │   ├── jsep.cjs.min.js.map
│   │   └── package.json
│   ├── iife
│   │   ├── jsep.iife.js
│   │   ├── jsep.iife.js.map
│   │   ├── jsep.iife.min.js
│   │   └── jsep.iife.min.js.map
│   ├── jsep.js
│   ├── jsep.min.js
│   └── jsep.min.js.map
└── package.json

3 directories, 16 files

They are also missing in the plugins.

JiriTrecak commented 3 years ago

@EricSmekens Just tried to download 1.0.2 to see whether the typing issue was resolved, but there is no deployed version 1.0.2 to NPM, it still shows as 1.0.1. I can see the package.json having the proper definition, however. It should probably be 🚢 ed :)

ammmze commented 3 years ago

@JiriTrecak I don't think its been published yet. They aren't setup to auto publish yet (working on it though). I believe @EricSmekens manually publishes when they are ready for a release.

6utt3rfly commented 3 years ago

I think @EricSmekens gave me permission to publish the plugin packages, but I'm still new to this project and prefer to defer to @EricSmekens for the release process. There's a github action for the main jsep release, but as @ammmze mentioned, nothing's been set up for the plugins yet.

EricSmekens commented 3 years ago

These fixes should now be published. (For now manually).

ammmze commented 3 years ago

I'm gonna close this issue. I think we are all good for now. Thank you everyone!

EricSmekens / jsep

Working towards the 1.0.0 release! #160