[compatability]: node + esm #1248
Comments
UpdateTook me over 3 hours, but i fixed it in my fork. I always forgot how awkward things get when i can't use the latest features. Here's the clipnotes version
why i'm not creating a PRI just don't have the energy to adjust the tests, and i'm also not going to update/remove all the outdated devDeps. it's just too much work, especially if i can't use the modern features i'm used to. I don't understand how anyone can work on a project when the following is the output of a simple install. I at least can not, and it is so excrutiating to fix things like these. My apologies for this short rant. Again i want to express that I'll gladly answer any questions about my fix or about modernizing parts of the project. And if maintainers are in favour of modenizing, I will gladly join in the work, i just won't do it alone only to find out the maintainers are not interested in upgrading because "it works just fine as it is". NOTE: |
|
Hello @KilianKilmister Thanks for reporting these Issues 👍 Friendly AdviceIn general I'd suggest less ranting, assumptions and judgment and more As a project that has been around for 5-6 years now, you should expect some level of legacy and modernizing needed, It does help when you point out these issues and link to relevant docs, and it would help even more if you can even assist in solving some of these issues. However random complaints/rants are non-productive Back to the Technical TopicsNamed ImportsI've been able to reproduce the issue by modifying the ECMA6 (commonJS) example. I will investigate this farther in the future, I am a little wary of the "exports" property I am also a little wary of your descriptions of the many changes needing to be made, I guess I will be wiser when I have time to dive deeper into it. deprecation warnings on npm install
fs-extra and node 14I am not sure there is actually problem. I also recently updated (via dependentBot) fs-extra from 9.0.0 to 9.0.1. regexp-to-ast libraryMy intent is to deprecate the uthe This is actually the highest priority next item I would like to tackle as with the ECMAScript regexp syntax being a moving target, more and more issues and gaps are being detected with the I am actually more interested in removing / extracting some less commonly used features to reduce the maintenance overhead of Chevrotain, as it is not a small library with ~9,000+ LOC (productive/runtime). Mocha a testsI am not familiar with mocha and ESM modules either.
IMHO the main issue with the testing is due to the historic mistakes of not encapsulating all test runtime logic Cheers. |
|
Apreciate the lenghty (and fast) response As there is a couple of things i would like to explain suficiently, I'm going to split it up, writing the more in depth parts in a separate doccument (probably on my fork, i'll comment a link in this issue-thread once its done.). And no worries, these are all just suggestions and opinions. Course of action is up to you. About the AdviceI'm not proud to say this, but this was the nicest formulation i could muster at that point (explanation following), i also want to stress that nothing i've written above was meant in any demeaning way (exept for the spite thrown at Mocha), and my apologies again. I'm very happy about your potential interest. I put a lot of effort in the issues i file, but i can't be spending too much time on a single issue, because i need to get back to my project at some point or discover other unrelated issues while exploring one. So i'm usually quite exhausted at the end and just want to get the report done. I pulled a few too many all-nighters in the past few weeks because of two projects that are very important to me. On one hand I'm involved in the npm-v7beta where im pushing heavily for a feature, for which i need to work on specs and prototyping dicuss it in meetings, the otherone is a new command-shell with JS/ES like syntax (which causes my interest in this project). Long story short: I need to take some more breaks. Enough rambeling, let's get down to buisness:Named imports
Well, it is a semver-major. but it also gives an author precise control over what he wants to expose. Using not explicitly mentioned entry points into a module also should pretty much forfeit any expectation of stability, as this is pretty much the definition of internal code. An example of a popular tool who was a very early adopter is rollup (over 2.3 weekly downloads). They included it in the release of version 2.0.0, and the had it working without any issues one day later and ever since. (original commit, fix 1, fix 2). I will reference them again in the seperate document when making the case for solid ESM support. From their current {
"exports": {
".": {
"node": {
"require": "./dist/rollup.js",
"import": "./dist/es/rollup.js"
},
"default": "./dist/es/rollup.browser.js"
},
"./dist/": "./dist/" // <- folder export
}
}
If we leave away the replacement of fs-extra (which was made on a convenience base only) and
All other things are not strictly nescessary. deprecation warnings on npm installI red the CONTRIBUTING.md, but i'm not familiar with it so i switched the nescessary configs to use npm. A lot could be discussed about deprecation and vulnerabilities, but I'll leave that for another time. I want to only adress a single point here: Lerna.
The thread covvers the detials, but right now lerna is in a limbo. Depending on what happens in zje next few weeks, it might be smart to drop it from the project. But i can't really suggest one way or the other as things stand. fs-extra and node 14fs extra doesn't really provide anything that node-v14 (or even 12 actually) doesn't have as a builtin. fs-extra is a bit more fool-proof, but looking at your detailed reply I don't think that you need much fool-proofing. I think it would be easiest to just drop it (it wouldn't be much of a problem to rewrite the scrips i made to be usable without top-level-await). I'l be talking more about node-v14 and related things in the other document. regexp-to-ast libraryMight not be a bad idea to replace it. I ran in to issues because i needed to use unicode property escapes as they are pretty great ( removing / extracting some less commonly used featuresBy the layout of the Workspace i'm assuming it's intended to be a mono repo anyways, right? I think it would be a great idea to split off parts by functionality into a few seperate packages, it really is a pretty large codebase for only one. But i'll also be talking more about that in the second document. TestsThe one i commonly see used is Tap, it's usable for both cjs and esm and is imported/required like any other module. It also doesn't cloud up controle over the test scripts. each file could in itself be a test that could be run alone with Depending on the tests to be run, it might also make sense to run tests against a server, passing the necessary information to it and validating the reply. Such a server is easy to implement on a socket/pipe/port and if applicable, tihs would solve much of the overhead issue. Similarly, using a docker container might be smart, but both of these are not very important right now. I can talk more about it if you want. What exact method makes sense will also depend on the potential splitting/shaving parts of the project This has gotten way longer than i wanted it to be, but so be it. It will probably be a few days before i got the other one finished. I'm planning to take a well needed break. |
|
@KilianKilmister no worries, and no harm done 😄 Named Imports.I am interested in solving this, however its not something I can tackle immediately. I think this may make the impact of an existing bug #1056 worse. It's fine to make breaking changes, as long as they are well documented. However sometimes its best to group together multiple breaking changes so that may also affect timelines. LernaInteresting that it is in limbo, But is there any alternative? fs-extra and node 14I had a quick attempt at removing fs-extra, and in fact it is only really needed for a
While its a little ugly to have the whole
regexp-to-ast libraryThis library is part of an optional optimization flow. So you won't be losing any functionality in Chevrotain removing / extracting some less commonly used featuresInitially this was a single repo with a single At some point I've refactored it to a lerna mono-repo with a single The next step would be to start splitting apart the functionality into multiple smaller packages. TestsIt seems like the esm source code is tested with mocha as well using the esm package wrapper.
I have been using Mocha in multiple projects for many years and I'm pretty fond of it. The concept in Tap of each test file being directly executable is very appealing. I wonder how Tap deals with TypeScript projects.
My even stronger preference is to avoid TypeScript code generation and only use TypeScript Cheers. |
Named Imports
I did check through the code but must have missed that. But either way, It's mostly stateless. but it's great you are pointing that out, as these things could case real issues if missed.
Of course. This should be included with changes that would result in a semver-major anyways Lerna
The short answer is no. The longer answer is: there isn't any tool that fits as broadly as Lerna tries it. the major package-managers, namey fs-extra and node 14I've been meaning to ask: any specific reason you used synchronous filesystem operations? Or are you just not used to their async counter-parts and wanted to be save? Also, as this made me realize i didn't have a function to copy a directory in my personal utility lib, i wrote some up a sync and async one and i wanted to do some benchmarks just for fun (and to learn the node API). sync vs async doesn't make much of a difference for the occasional use, but async tends to be somewhere close two twice the speed. but this isn't an urgent thig, so it's not too important. teststhe ironically, if you want to use it with tap you have to set its options to I never used webstorm, I'm using vs-code. it also has things like these, but i don't know how much they differ.
with the native loader all you need to do is start node with: node --loader ts-node/esm path/to/file # with typechecking
node --loader ts-node/esm/transpile-only path/to/file #just runon some unix distros you can add command arguments to the hashbang (macOS works, linux doesn't for exampel), so i just have the loader arg in the hashbang. eg: I'm running both proper tests and prototyping stuff with it.
Since i transpile to esNext and esm, the resulting JS/ES code is pretty much exactly like the source with all the type information stripped away. And this works for small things just aswell to be honest. i mean i write my build scripts in typscript, too. i'm currently trying out const enums, since they are transpiled away, but are really great for documentation and make consistent changes easier across the procect.
With a project architecture like i'm using, the difference is about 1-3 extra seconds spent on transpiling before any run (being done automatically). this becomes negligable in my opinion considering all the added safety. Both in classic repos and in mono-repos. For example the following TS source file file from a small package in a mono-repo of mine containing a few files. (it's by far the largest file in that package) import { Null } from './null.js'
export interface TriggerGroup extends Trigger {
children: Trigger[]
}
export class Trigger<F extends (...args) => any = (...args) => any> extends Null {
#isArmed = true
get isArmed () { return !!this.#isArmed }
trigger: (...args: Parameters<F>) => boolean
disarm: () => boolean
constructor (action: F) {
super()
if (action instanceof Trigger) return action
this.trigger = (...args: Parameters<F>) => {
if (this.#isArmed) action(...args); return this.armed
}
this.disarm = () => { const state = this.armed; this.#armed = false; return state }
}
static makeGroup (...actions: Array<() => any>) {
return this.link(...actions.map(action => new Trigger(action)))
}
static link (...triggers: Trigger[]) {
const triggerGroup = triggers.filter(trigger => trigger instanceof Trigger)
const trigger = new Trigger((...args) => triggerGroup.some(child => child.trigger())) as TriggerGroup
const { disarm } = trigger
trigger.disarm = () => disarm() && triggerGroup.some(child => child.disarm())
trigger.children = triggerGroup
return trigger
}
}Turns into this JS/ES file. this makes it explorable in the transpiled version aswell, as all the doc comments would be included. just one of the reasons why esm is amazing. import { Null } from './null.js';
export class Trigger extends Null {
#isArmed = true;
get isArmed() { return !!this.#isArmed; }
trigger;
disarm;
constructor(action) {
super();
if (action instanceof Trigger)
return action;
this.trigger = (...args) => {
if (this.#isArmed)
action(...args);
return this.armed;
};
this.disarm = () => { const state = this.armed; this.#armed = false; return state; };
}
static makeGroup(...actions) {
return this.link(...actions.map(action => new Trigger(action)));
}
static link(...triggers) {
const triggerGroup = triggers.filter(trigger => trigger instanceof Trigger);
const trigger = new Trigger((...args) => triggerGroup.some(child => child.trigger()));
const { disarm } = trigger;
trigger.disarm = () => disarm() && triggerGroup.some(child => child.disarm());
trigger.children = triggerGroup;
return trigger;
}
}
//# sourceMappingURL=trigger.js.map |
|
While I have no association with Lerna, I would indicate that one way that could help keep that project alive would be to have additional contributors help out there. I'm not sure how the primary maintainer would like to manage that, but, I suspect getting assistance on PRs and issue fixes might help based on the limited emails I've had with them. That being said, PRs and issues seem to go for months without any feedback, so, I suspect it will require someone to take it over completely, at some point. |
|
@MikeActually I've read through the relevant discussions about this a few days ago, and we will have to see what the result of the dispute claim will be.
I can say right of the bat, until something changes that, i'm not going to do any work on lerna. I'm going to continue this in your issue thread in the lerna repo because it's not the topic of this thread. |
|
Only wanted to let you guys know that node now got a experimental specifiers flag so you do not need the file extension https://nodejs.org/api/esm.html#esm_import_specifiers |
|
@frank-dspeed |
|
Back to working a bit on OSS this weekend 😄 Lerna
I used to write my own scripts for a pseudo mono-repo. replacing them with Lerna + yarn worked much better 😄. Scripts
In the context of small dev scripts that work on small data sets it is the most simple approach.
Project Structure and ts-node@KilianKilmister: I did not understand from your reply how you got ts-node to work with TypeScript project references and/or incremental builds.
With project references and incremental build I am getting ~1 second compilation times for small changes which is pretty good:
Regarding the reduction of the abstractions. However all abstractions may be leaky at some point. And I tend to regard I even regard TypeScript as a sometimes leaky abstraction, which is why in a small project I may consider This is of course a subjective opinion. 😄 |
|
@bd82 But this isn't really that important, as long as a working way exists.
Hearing things like this make me really happy, can't wait for the day were IE11 will be nothing but a memory.
That's true. "the lesser of two evils" is also subjective of course.
Sure, i think that's the right thing to do for the time being.
I wanted to benchmark my own functions against Is there anything we talked about for whitch you would like to see a mock-implementation or PoC? We talked about pretty much everything i wanted to discuss except for some very specific technical aspects, which don't really need discussing unless they become relevant. |
|
closing this, will be tracked in: #1383 |
currently trying to use named imports with chevrotain throws an error:
Node will also only see the non-esm version since it ignores the
modulefield in the package.json.There are two fixes for this: conditional exports, nested exports (links to the docs).
And in either way, there needs to be a
package.jsonwith the property"type": "module"for node to recognize it as an ESM somewhere above it in the tree. (eg. in the tsoutDir)Lack of this also effectively causes the typedefinitions to be incorrect, as they claim the package does have named exports
Additionally, setting the TS
targetfor esm toes5makes little sense, as there is no esm in es5. it should be set to at least es6, since that would also mean that the outputted files use those es6 features that help static analyzers.EDIT (VITAL): node also requires file extentions to be added to import specifiers (follwing esm-spec)
The text was updated successfully, but these errors were encountered: