* 1* * * * is valid (* random places = valid)
mortenmoulder opened this issue · 12 comments
I have a few expressions that are absolutely invalid, but are reported as valid:
1* * * * *
1*1 * * * *
* 1* * * *
* 1*1 * * *
* * 1* * *
* * 1*1 * *
* * * 1* *
* * * 1*1 *
* * * * 1*1
See the pattern? If I instead start with * but end with 1 (example: *1), it's invalid in both crontab guru and your validator, which is good.
Thanks
@mortenmoulder nice catch! The reason seems to be that parseInt('*1'); return NaN while parseInt('1*'); returns 1. So when we check if the value fits in the range, we get the number alone so it's considered valid.
I'll use a custom parse function based on this https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt#A_stricter_parse_function but with only positive digits accepted.
Very quick execution! I like it. Thanks a lot :-)
I've got a question: Since I need this to run on a pretty static website, would it be okay if I convert this to ES5 compatible code and use that? I know it's MIT licensed, but figured I might as well ask. I can't use the TypeScript source for this project, unfortunately.
@mortenmoulder The published code is ES5 compatible. I publish the lib, not the typescript version.
@GuillaumeRochat Yeah, I get that. My problem is, there is only a TypeScript file. I need .js file I can safely inject into a <script>-tag. I need it to be very backwards compatible with old browsers, as well.
I simply copied the contents into https://www.typescriptlang.org/play/ and selected ES5. Then I am able to literally copy-paste that converted JavaScript code into its own file and use that on the page. While I know that also exposes the other "internal" methods, such as your new safeParseInt function, I think it's an okay approach, so it can be easily updated in the future as well.
@mortenmoulder Are you using the source from npm? From npm, there is only a js file, no ts file. If you aren't, then I suppose you are free to do as you want, since it's MIT. If you can find a way, use dependecies from npm instead and bundle that. This way you will have an easier time to update without manually doing everything every time.
@GuillaumeRochat I am not using it via npm, no. My solution doesn't have anything related to packages, and I'd rather not introduce it just yet. I cannot find the .js file on npm (is that even a thing?).
What I've seen people do (not saying you should, but it would make a lot of sense), is to include both the how to install via npm and how to install a minified version directly into your browser. Then there's a src folder (like you have) and a dist folder, that includes the compiled ES5 compatible version, that can be included on the site with a <script>-tag.
I think in general it's a bad idea to release the source code for something, without including the compiled stuff as well. If I wanted to use your package on my site, I would have to do something manually, if I didn't want to use npm (as explained in my last reply).
Again, I'm not saying you should! do this, but I figured I might as well tell you my perspective, because I think this is a really nice package, that does exactly what it should. Nothing more, nothing less.
@mortenmoulder Ah ok, I see, I think this should work for you for now : https://unpkg.com/cron-validator@1.1.1/lib/index.js
All the npm published packages are available on unpkg like that.
@mortenmoulder Oh wow, I also noticed that : https://www.jsdelivr.com/package/npm/cron-validator It's automatically minified and bundled on jsdelivr.
@GuillaumeRochat Ah, that's pretty nice!
One problem is, that it still uses exports. While exports are great for TypeScript modules and NodeJS, they are the exact opposite for vanilla JavaScript for the browser :( I would get exports undefined errors all over the place, unfortunately.
@mortenmoulder Oh right! You could clone the repo, try changing the module line in tsconfig.json to UMD and do a npm run build. This should give you a umd version in lib/index.js that you could import easily, though it wouldn't be on jsdelivr/unpkg.
@GuillaumeRochat Definitely, yeah. That would be a good way, but a better way would be to include a <script>-able library. However, with how you wrote the library, it would expose all the other functions that technically aren't necessary. So it might require some rework, if it's supposed to satisfy everone haha.
Anyways, great talk! Thanks for the quick update 🤘