A simpler way to detect Preact
molefrog opened this issue ยท 5 comments
Hi! I was doing some digging into the source code and I noticed that the current approach of detecting Preact environment creates some constraints in the other parts of the library. If we just had some simpler, ideally synchronous way of Preact detection we would have:
- Avoided having an invisible
span
rendered by<FpjsProvider />
. While it doesn't seem like a big deal, there could be some clients that expect the library doesn't produce any html, and could rely on some exotic CSS in their apps likebody > div
. - Simplified the client initialization and loading logic and potentially avoided extra rendering at the startup.
I don't have a definitive answer yet, however I have compiled a list of tricks that might work here:
- A more hacky and less stable option:
ctx = createContext({}); isPreact = ctx._id || ctx.__c
(the property name is_id
in the source code, but it becomes__c
after mangling). Pros: works for Preact starting from 10.0.0 (the first release where hooks became generally available), doesn't require any additional component, completely synchronous. Cons: it's implementation specific and might break in the future. - Check the number of arguments passed down to class component's
render()
. Preact always has two agruments. Pros: works in both 8.x and 10.x, it's an official documented difference and it is less likely to be changed in the current release Cons: it is not completely synchronous, however it doesn't output an html element.
Let me know what you think,
Cheers.
Hey @molefrog, thanks a lot for your suggestions!
I've tried them both, and the second one seems very solid, so we will definitely try to use it in our implementation.
The first one also did work for me, but we want to avoid solutions that might potentially break in the future.
Cheers!
Hey @molefrog, thanks a lot for your suggestions!
I've tried them both, and the second one seems very solid, so we will definitely try to use it in our implementation. The first one also did work for me, but we want to avoid solutions that might potentially break in the future.
Cheers!
Fair enough. Regarding the second solution, I've checked it as well and unfortunately there is no good way to make it synchronous. It also requires a class
being declared, which considering that we target es5
could result in a bigger bundle. However we could avoid classes with this trickery: I've made a demo to illustrate that.
we will definitely try to use it in our implementation.
Is there a possibility for me to work on that PR? It would be a fun micro-project for me. I'll probably need some review and assistance though with the code style etc.
Is there a possibility for me to work on that PR? It would be a fun micro-project for me. I'll probably need some review and assistance though with the code style etc.
Sure! When you submit a PR someone from our team will review it. If you need help regarding how to setup the repository locally, you can follow the contributing guide.
Fair enough. Regarding the second solution, I've checked it as well and unfortunately there is no good way to make it synchronous. It also requires a class being declared, which considering that we target es5 could result in a bigger bundle. However we could avoid classes with this trickery: I've made a demo to illustrate that.
That's a good point!
Regarding it being asynchronous: it's not a big deal, because our current solution is async as well.
Regarding requiring the class to be declared: in the case of my test code I ended up with something like this:
Build:
var Cmp = /** @class */ (function (_super) {
__extends(Cmp, _super);
function Cmp() {
return _super !== null && _super.apply(this, arguments) || this;
}
Cmp.prototype.render = function () {
var args = [];
for (var _i = 0; _i < arguments.length; _i++) {
args[_i] = arguments[_i];
}
console.log(args);
return null;
};
return Cmp;
}(react.Component));
And the original code:
class Cmp extends Component<any, any> {
render(...args: any[]) {
console.log(args)
return null
}
}
The console.log(args)
showed a correct length of arguments in react and preact. We can however compare the size of the bundle before, and after implementing it just in case.
I've also noticed one interesting thing - in my case in Preact I've received 3 arguments in render()
function, the last argument was our context. I think that it's worth keeping in mind when you will be implementing it.
Thanks!
๐ This issue has been resolved in version 1.4.0 ๐
The release is available on:
Your semantic-release bot ๐ฆ๐