-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Discussion] Is an ES5 build/distribution necessary? #2642
Comments
Also, I'm very curious how others have solved this problem (if anyone has it) as I assume there are indeed ways for consumers of our NPM library to run babel against it themselves during their build process and ES5-ify it. I'm not sure how hard or easy that might be... |
To be clear right now the main blocker is that this does not seem necessary and I'm imagining there are other reasonable solutions (which I hope someone will come along and mention). Not to mention that if browserify wants to remain relevant I'm sure it will be adding proper ES6 support - am I mistaken? So it only seems a matter of time until this resolves itself - without requiring any time on our side. @puzrin Have you filed an issue with browserify on this or does one already exist? A link to it here would be great. |
No. I have no concrete suggestions to solve for them. |
Well isn't the problem simply that their build pipeline needs to properly support ES6 code? This seems like an issue that should be filed against their project. What am I missing? This is one (of many) reasons we moved away from our prior build system... it could not understand ES6 code, so it was a poor choice since we're now using ES6 code in the project. |
They have option to force babel for all sub-deps (see faq link). But that increases build time. I have a lot of deps, but only hljs has such "problem".
IMO this things are not related anyhow. Right now you can bundle multiple esXX versions, with almost no efforts. May be later, if you decide use advanced es6 feature this may change. But right now drop of es5 looks very strange decision. |
To be clear we didn't "drop" ES5 - so much we upgraded from ES5 to ES6. We've always supporting a SINGLE ECMAscript standard - now we simplify support ES6 rather than ES5.
Sounds like it shouldn't increase build time much if it's only library that needs to be "down-sampled". And with a good build system there should be ways to cache this as well...
You didn't answer this... our build system (Rollup) works just fine with ES6 code natively... what is the problem with Browserify that it can not bundle ES6 modules? |
@puzrin Please see issue I opened against
Perhaps it's your transforms, not browersify that's the problem... or you need to install acorn-node? |
acorn-node is used internally by browserify so there is no need to install it separately. From the other issue it sounds like the actual problem is not syntax support in browserify but the fact that it has no simple way to transpile a particular dependency. You can combine https://www.npmjs.com/package/tfilter and babelify to do that. Babel itself filters out node_modules files at some stage I think and needs some configuration to avoid that, but I don't remember what exactly. |
Why would our library need to be transpiled at all if the syntax is not a problem or browserify? |
Only if you do want to target old browsers, or if you are using outdated transforms. |
You ask me question, that requires dig browserify docs or debug existing app. That's a bit against of my goal. I don't keep in head everything. I have big project, with es6 cjs sources + es5 deps, and with es5 output after bundler pipeline. It's ok for my need, and i would not like to change it until possible. |
@puzrin Looks like this is something you'll need to fix yourself. Seems clear this is specific to your project (or at least fixable if you decided to put in the time) and not truly an issue with Also: You can always build HLJS separately and just use the pre-built JS file outside of your complex build process completely - or just have your build process "concat it" (without any processing). Most build pipelines have an easy way to do that. So it seems there are numerous ways to solve this easily - even if not the exact way you'd like to see it solved. Good luck. Closing this issue. |
@joshgoebel it is, quite simply, unsafe to transpile code one didn't author, so by your choice to support ES6 and not ES5, you have effectively blocked anyone from using your library in an engine older than your native syntax. I would strongly urge you to transpile your package to ES5 before publishing if you are interested in those users being able to use your library. |
@ljharb Curious, how is it "unsafe"? I'm not sure I was recommending that in any case. We no longer support any ES5 browsers so moving to ES6 on the client-side made complete sense. I offered several suggestions for getting around this persons legacy build system that they don't want to fiddle with. Simply loading the JS file separately or "concat"ing it "as-is" to a larger build should work just fine for any browsers we still support. |
@joshgoebel transpilation is not 1:1, there's all sorts of caveats and edge cases that only the author of the code is qualified to determine if they don't apply. You're certainly within your rights to drop support for ES5 engines - but please do so knowing that it is effectively impossible for anyone who supports those engines to use your library any further. |
Gutted you've dropped support for ES5. I just added your code to a project that has a gulp build system, the es6 code in your lib breaks that. How would i build an ES5 version of this? |
No idea - but I'm 99% sure that's the wrong question. We no longer support ES5 browsers and JS run-times. If Gulp does not support ES6 easily I'd suggest perhaps just dropping the web build of the library directly into your project (in its entirety). IE:
Of course you'd likely automate this... (or just make bumping the dependency a more manual task). Of course you could also choose to just serve the Even a simple one-line concatenation can work: cat highlight.min.js your_monolithic_js_bundle.js > final_build.js |
This is very untrue and also misleading as there are any number of ways to include our library still (I just mentioned one to @john-doherty)... it's definitely still doable, perhaps just a tiny bit more elbow grease or perhaps none at all (if you're willing to just load the asset separately as MANY websites do - including some very large ones). |
That's only the case if highlight.js is a direct dependency (and not a transitive one), and also if your build process allows for that kind of alternative inclusion (most modern ones don't; everything comes in as a normal npm dep, and is run through a bundler). It's certainly doable - it's computers, we can make them do anything that can be done - but it's effectively impossible because it's not going to be practical to safely do so in the general case. |
I'll qualify here: any build process/system that can't easily concat two files together is already a lost cause. :-) I know for sure Gulp can easily do this because I've done it before. If the complaint is really "but i have an extra build step" (Highlight.js) then yes, you would (or you could vendor the library as a static file) - but this should be an easy thing to automate in a reasonable build systems. If not I'd argue the build system is deficient.
This is a much better a point and arguable a bit harder to solve (I'd imagine). I imagine it would require a small patch to whichever dependency pulled in HLJS to use a the global ES6 version instead. Obviously would be situation dependent. My opinion here remains the same. This seems to be a niche problem destined to solve itself as ES6 support becomes more mainstream - or people switch away from broken build systems that don't support ES6 (ES2015). It also seems there are ways to do this (with various build systems) just run everything thru Babel, etc, but I don't know about all that. It sounds pretty awful to me to have to do that when your deployment target is a modern browser. |
@ljharb Also (to clarify), often it's not even the build system that's at fault but exactly how people have used/extended/customized it (as so often happens with build systems). For example the original poster here was using Browserify but when I posted an issue upstream asking them if they support ES6... they most certainly totally do, but some specific transforms/plugins may not. So I have a feeling the issue here is often specific legacy build pipelines (as implemented by the user) rather than legacy build systems (overall). ES6 is no longer a shiny new toy anymore. (it seems major browsers have supported it for 4+ years now and counting) |
In browserify i had to enable IMO your talks about build systems look like if i start teach you js programming - not very useful and not very relevant. |
Please, no need for rude or disparaging comments. I don't believe I've been disrespectful here. If "abused" came off poorly I've removed it from my comment (and you have my apologies) - I was using it only in the sense I hear it used regularly "use and/or abuse" in various context - not as any form of personal slander. I've personally used (and perhaps abused) several of the popular JS build systems (Webpack, Gulp, Rollup) on different projects and I've written the build system we use with Highlight.js entirely from scratch (though it uses So far though I feel I've been mostly correct here in my overall assertion:
So a super easy solution, yet one that does indeed have a small cost. So not "effectively impossible". |
Based on years of client experience I never solve the stated request... I focus on underlying problems. In my experience 95% of the time the stated request/desired solution is wrong (or incomplete). Most often the underlying problem has not been correctly understood.  Correct, right now there's no intention to provide an ES5 build (just to run on ES6 runtimes). I'm unpersuaded anyone needs that build - including you. 🙂 It would not surprise me if the solution for Browserify is a small amount of JS code to customize the build. Yet it's clear I don't yet understand the underlying problem so that makes it harder to help.  If your target is an ES6 runtime then there's no need to transpile our library and there should also be no need for an ES5 version. It's just the wrong solution. Makes no sense.  I assume your profit would be your build times returned to 30 seconds. 🙂 But if your done discussing that's fine.  |
It might also help if you clarified your problem a bit more. How exactly does our code break the build? Are you saying that Gulp simply doesn't support ES6 code? Can you show us your Gulp set up? Edit: I've used Gulp with ES6 code before without issue. |
Most build processes do not transpile third-party code, because it's not safe to do that. For example, https://unpkg.com/browse/[email protected]/lib/core.js has arrow functions - it seems like the way you are using them, they're just syntactic sugar, so they'd be safely transpilable, but arrow functions themselves are not safe to naively transpile. They lack a Any build process that blindly transpiles third-party code is being reckless and risking breakage to the most vulnerable of users, those using older browsers/engines. It's certainly something that can be opted into, but it is not safe and it's not reasonable to expect anyone to do it. In other words, if you want to drop support for runtimes that, for example, lack arrow functions, please accept that you are forcing your consumers to do the same, because it's simply not practical to audit the entire dep graph and determine whether transpiling is safe, since the vast majority of transpilation is not 1:1. |
https://github.com/highlightjs/highlight.js/blob/master/.eslintrc.js#L17 Linter allowed settings are es9 (es2018), not es6.
|
@ljharb Did you read my last few messages? I'm not asking anyone to transpile us... I'm suggesting there should be zero reason (or need) to do so. We only support recent versions of Node and green-field web browsers - all ES6 run-times. Most anything in the past few years works just great. What puzrin was seeming to suggest originally is that he only needs an ES5 build to make his build system happy (because it chokes on ES6), but then when he then starts talking about running us thru Babel I get lost.
Out of curiosity... how does that make this "unsafe"? Or are you simply saying because the behavior (in edge cases) can't be 100% guaranteed that it's therefore "unsafe"? I can't think of why someone would call new on an arrow function or access If someone were going to transpile us they'd obviously they should take the security concerns into mind, but I just don't see why anyone would...
We've been VERY clear about that. Do only support ES6 run-time environments, and the library we publish therefore is free to use ES6 features as it sees fit. |
Indeed. Though this does not require an ES5 build. Though it's potentially a valid reason one could want an ES6-only build. We currently only actually use ES6 code (AFAIK), but just looking now I'm also not sure I have any issues with us making the minimum requirement ES2018. All the desktop targets we support seem to well support ES2018 as well (at the least the features we would use, I'm not sure about FULL support). I'd have to take a closer look at mobile first.
It would be helpful if you mentioned specially which environment you're trying to target - which browser are you actually worried about? Desktop? Mobile? (or do you not really have one in mind?) I think you're supporting a wider range of browsers than we support... Off the top of my head it'd be something more like:
An ES2018 vs ES6 discussion might be a much more relevant topic. :-) |
@joshgoebel yes, but in reality, there is a lot of need to do so. You don't have to agree, of course! However, anyone whose business requires them to support these browsers - which is a great many of us - simply can not use your library without it providing an ES5 build. I agree that anyone targeting an ES6 runtime doesn't need an ES5 build - the problem is that most of us absolutely must target an ES5 runtime, and forgoing the revenue from those users is simply not a viable option. So, again - not providing an ES5 build screws over your users who must target ES5 runtimes. If that's the choice you're sticking with, so be it, but please do not pretend you're doing anything less than that. |
I don't think we're doing any pretending. We very purposely chose to drop support for IE11 and other non-ES6 browsers. We simply don't have the time (or desire) to support them or their weird quirks. We're supporting 95-98% market share (and growing of course) according to I can certainly sympathize with business requirements, but someone who needs to support IE11 should probably look for a different highlighting library - or perhaps maintain a small fork (or sponsor one) where they are then responsibility for old browser compatibility and breakage, etc.
Just curious, is there a specific runtime you're thinking of other than IE11 when you say this? The fact that this issue (and dropping support for IE11) have received so little interest leads me to believe we're making a reasonable choice for most of our users. Version 10 was released at the beginning of the year and only one or two people have spoken up about IE11 or older browser support. |
Browserlist readme advices to worry about
As i said earlier, i find your opinion about using bundlers in outer projects too obtrusive, and discussing such things - not optimal time spend to solve introduced problem. |
I don't know about some of these mobile? options but if you remove IE11 from that list I think there is a lot of ES6 support there. Everything I recognize at least is quite new (all the major browser players, etc) and supports ES6. Opera Mini seems a no and both Baidu & KaiOS are listed as "some compatibility". All of those being < 1% usage.
Not sure I follow what you're suggesting here.
Just to confirm the existing codebase is all ES6 [no ES2018 code according to linter] (and likely to stay that way for the near future). So if you were only running us thru Babel because of that mention of ES2018 in the eslint config, you can stop now. :) |
For clarity - i don't think you MUST support es5 build. I solved my needs silently, without BS like "you don't understand how opensource works and should not ignore needs of project users". I only don't like mutating context of issue into direction "such demands are silly" (with meaning "silly" demands are written by "silly" users).
I just suggest to be more gently with public statements :). What you feel with this is ~ the same as i feel with your posts about "how bundlers should be used" :).
See above. I don't like idea to customize bundling process for every specific package. That's not good for my maintenance expectations. |
Except you just kind of went and said it now... I'd suggest "ignore" is a very strong word. Everyone on the core team volunteers their time to maintain this project. We support 95-98% of browsers commonly in use today. I spend a lot of time on this project meeting a LOT of users needs. I'm sorry if you feel your projects needs have been ignored, but let's not overly generalize.
I don't think I used the word "silly" anywhere. I think the language barrier here hasn't helped us any... it's possible you're reading tone or intent into what I'm writing that simply is not there. I tend to be very plain spoken.
I don't believe any of my remarks about bundlers devolved into personal insults. It probably would have helped if I understood your original goal better (way back when). To me the only non-ES6 browser that even exists (and possibly worth caring about) is IE11... If that makes me dumb, then so be it, but the market share of any others (overall) seems so close to 0 as to almost be a rounding error. So your talking about ES5 apart from IE11 really just went over my head (my apologies). For a long time I thought you wanted to build ES5 just because your build system mandated it for some strange reason - which made very, very little sense. :-) Secretly I wondered if you really still just wanted IE11 support (else why compile to ES5? lol)... but now in re-reading it's clear what you were really saying was "Can I still use this in all my other ES5 browsers, just not IE 11?" To which the answer is still no, the market share is just too small to make them worth supporting. Without using or knowing anything about those niche browsers it's simply impossible to make sure users would have a great experience. So along with IE11, they are also now unsupported.
I wasn't being literal. I was only pointing out that I think most the browsers on that list (with any real general usage) already support ES6, with the exception of IE11. If you were able to drop your IE11 and Opera Mini requirements you might be able to get away with an ES6 build. (not sure, but maybe)
Sure, if you're happy with 50s cold start instead of 30s cold start then I suppose it's "ok". Like most things, it's about tradeoffs. In this case the simple "one line fix" doubles the cold start time... vs a slightly more complex bundle that would keep the fast cold start.
I think I've explained a lot... perhaps I missed a question? I also don't see where I've used the word "clever" anywhere. And not sure what I've declared ancient... ES5? Generally, for 95-98% of browser share it is... though to be clearer relevancy might be a better metric than age. |
@puzrin You might also be interested in #2756. It's only a matter of time until we're using ES6 features you can't transpile to ES5 at all... and then you're looking at maintaining your own fork of the library... and that's also why we don't officially support ES5 anymore - it's about to potentially get much harder to do so. Lack of time, lack of team interest, lack of real demand, tiny (and shrinking) market share, and we know that we're likely to break it in the near future anyway. You're welcome to transpile it as long as you can (if that works) - we certainly can't stop you. But we didn't randomly drop support for ES5. It was considered and it was a signal of where things are headed... so while transpiling is working for you right now I'd be spending some time planning your next move... If IE11 (and other ES5) support is mission critical for you I'd seriously consider looking at https://prismjs.com/ which so far seems interested in IE11 support. Nope, never mind, they are looking to drop IE11 (and likely ES5) support soon as well. PrismJS/prism#1578 |
Again, i'm ok if you ignore demands of es5. I'm no ok HOW you do that. Probably, due language barrier i can't express my thoughts good enough.
Since i have low telepathy skills, i react not to what you think, but how i understand what i read :). From my point of view, package demands could be isolated. Propagating those to app requirements (discussing bundlers) is out of scope (IMO).
IMO operating with guesses is not constructive. There is wide-used browserslist "default" preset. You may not like it or disagree with it. But some people prefer rely on it without care about details of each browser.
Those comment was given in context you were giving advices without knowing details and side effets. Any your decisions about hljs are ok. But attempts to "intrude" into outer processes can make someone feel uncomfortable. No, i'm no "happy" with increased cold start time. I'm just "ok" with it. And if i decide to spend time for improve, i will try multithreading support instead of written recommendations.
Missed question is, that i had to spend notable time for bundler changes and talks without reasonable needs, from my point of view :). You forced users like me to participated in hljs process, instead of allow make a choice, what kind of bundle to use. That's too pushing. At this moment, bundling to es5 is completely automated process with almost zero cost for you. |
When you use non-transpileable es6 features, that will be another story. But right now you compare "not existing potential features" with really existing "difficulties". IMO such kind of arguments is "black rhetoric" (when goal of discussion replaced with goal to prove "i'm right and you are not"). |
Obviously you're understanding me wrong. I was simply giving you a clear heads up. You seemed to feel taken by surprise when ES5 was removed (in a major release). So despite you're setup already being unsupported I'm giving you warning now so when things break you can't say it came as any surprise. You could be doing something about that today vs waiting for it to break tomorrow. But obviously that's your decision. :-) Justifying why we dropped support for ES5 isn't the same as claiming "I'm right". It's possible a different maintainer (in another universe) would have made an entirely different decision. It was the right decision for our team I think, but I don't think it's right or wrong in any universal sense of the word. It simply is what it is. |
Now that I clearly understand your goal I probably would not have said much of what I said and simply stuck with "Sorry, ES5 is simply not supported for IE and other browsers." :-) Oh well, water under the bridge.
Every suggestion is a guess until properly researched, no? :-) I was merely pointing out that browserlists own "default" preset itself seems to mostly agree with our policy (once you remove a few outliers) to drop ES5. When it will finally match completely I obviously have no idea. I'll avoid guessing. :-)
I offered some suggestions - most people appreciate this. You may take them or leave them. There is no reason to feel uncomfortable.
That's an idea! :-)
No one is forcing you to change your bundle setup or forcing you to participate. You're free to leave.
So it's ok when you make the suggestions, but not when I do? :-) Seems a bit unfair. :-) I've already addressed this zero cost myth. Implementation, maintenance and support costs are real costs. Time it takes to make hotfixes to deal with stupid ES5 only bugs in older browsers is real time that we don't get back. Asking people "are you using the ES5 or ES6 build?" makes diagnosing and triaging problems harder. "almost zero in a perfect world", maybe. "almost zero" in the real world in 2020, definitely not. |
This is past the point of being constructive though. You will continue to make the decisions you feel are best for your project, and we will also do the same. If there are any final thoughts lets air them then return this thread to resting in peace or perhaps helping @john-doherty if he still needs assistance :-) |
I have a bit different long-term plan. Sate of browsers is not static. When es6 become really mandatory in hljs, browserslist may give complete different result, and changes may be applied wihout pain. Strategy of "doing nothing until possible" looks more attractive for me.
Weight of suggesion is direcly related to "who will pay for it".
Logic is simple and honest, IMO :)
My suggestions are based on my experience. I respect your time and would never suggest "difficult" unpleasant things. See how many project i support in my orgs. IMO i have reasonable stat about transpile to es5. If question would be only in doing PR - i could do that. I suggest a minimalisic thing - completely automated es5 build without support, "use on your own risk and not with ie11". That could be enough until better time come. And of cause, i'm ok with simple answer "i don't like to support es5 build" :). In my eyes personal preferences are normal and don't need technical proofs. |
Hi @joshgoebel, you're right that gulp was not the issue, it's more the gulp js minifier I'm using. The es6 only option is a blocker in my case, unless I was willing to do more than copy and paste it into my project. I did not have time, so went with prismjs. I think that's the concern here, it's a potential barrier for new users - which could hurt future adoption. I don't know the codebase, but cant you just add a minified es5 version? |
👍🏼 There are definitely some real merits to that approach for sure. :-) The UTF-8 stuff might land in Dec/Jan/Feb timeframe. We'll see how it goes.
That is a good point. It would carry more weight if you were on the core team though and also required to deal with support issues, fixing ES5 bugs, supporting these obscure browsers, issuing hot-fixes, etc... I find that all too often the PR is only the "tip of the iceberg" when it comes to the actual effort of getting a thing done. Perhaps your experience is different. I think personally I'm much less comfortable with "use on your own risk" vs "it works on modern browsers, guaranteed". Obviously a matter of personal taste and perhaps OSS philosophy in general. We're not preventing anyone from transpiling (or even using IE11). We're just saying "it might turn out poorly, so we're not going to assist". Even if I were more receptive we already dropped ES5 support in April. We have a plan to use ES6 features soon that (afaik) either make ES5 not transpile not possible or require even more developer effort to maintain. So adding it back with the caveat "use at your own risk" just to take it away in a few months makes very little sense.
Well, but I also think our reasons are sound, even if you happen to disagree or think it's the wrong choice. It's a decision made with a reason, not just a whim. :-) |
No. See entire discussion above if you want more context. Supporting legacy build system plugins (when there are good alternatives available) isn't a priority. FYI: There are Prism issues talking about dropping support for IE11 and ES5 also. (though I dunno their thoughts on transpiling)
FYI: Terser works great with Gulp and fully supports ES6. Just FYI. We use terser for our own builds of Highlight.js. So if you decided to switch in the future you could do so easily with Gulp + Terser as your minifier.
Prism.js is a great project with much to recommend it. :-) |
ftr, that prism issue you linked hasn't had much discussion in the last 2 years, except to suggest using a build process to preserve support for those environments. |
I edited my last post to remove the "soon" and make it clear I'm not speaking for the Prism team. I didn't pay much attention to the dates more so than the overall direction... that ES5 is on the way out. I'm also not sure the age of the issues in this case is a positive sign. :-) As I said they might be more willing to transpile - though again I'm not 100% sure why if they decide to drop IE11 support. There were a bunch of interconnected issues about the future of Prism - I'm not sure if you looked at them all. Some would really like to user regex look-behinds (as would we)... that of course will certainly break ES5, though that's likely still a year or more out due to Safari's lagging support. |
I saw it had regular commits and was still use by Stripe so grabbed that... If this was my project, my concern would be that dropping ES5 would eliminate this tool from been used by any SaaS/API company that wants to sell to large orgs (many of which are still running old browsers/devices). |
@john-doherty It's possible there might be room for a
I think the ecosystem has room for more than one alternative - with different alternatives having different goals and focus points. We're better at some things, Prism is better at others - like being tiny, and supporting IE11. Users can choose which things they value more. We don't have to solve the highlighting problem for every last person, organization, and browser on the planet. We're ok being Apple and removing the floppy drive first. :-) We're happy solving (and well supporting) the 95% case. Perhaps for such a company Prism.js would be a better fit. Or perhaps if they are truly a profitable SaaS business with a dev team and a support staff then they could maintain (and support, and open source, etc) their own ES5 port of the library. It's not (currently) an insurmountable task (by any means) - it's just going to get harder and harder as time moves forward. |
https://babeljs.io/docs/en/babel-plugin-proposal-unicode-property-regex Wow, truly, people are ingenious... though the output of something like |
If you mean Usually, there are possibilities to keep acceptable compatibility (not full, but still useable) with es5 transpiler without pain for development process. If you need help with this - feel free to kick me anytime with questions or PR requests. |
There are no serious perf issues. I'm author of this madness, and it works well. Not sure if 100% can be transpiled (problem is negative classes and But, for this package i would prefer throw exception instead of bundle size increase. Then the only case to fight is syntax errors on script load. Workaround is to use |
Editor: Adding some answers here at the tope for anyone else who finds this with Google.
If you're using the library as a direct dependency
There is no need for an ES5 build if you're using the library as a first-order dependency. Just build us separately and drop the web build of the library directly into your project. (either as a separate JS
<script>
tag) or concat it to the end of your monolithic build. Simply:And of course you can do this easily with a build system as well.
If you're using Browserify
Evidently you can simply enable the
global
option. [more details would be helpful]The original issue:
Ref: #2501
As you know, v10 announced stop support of IE11. But, indirectly, published releases become not es5-compatible any more. If you have cases where es5 (babelified src) is still needed (except IE11) - please post here details.
Support of es5 may be considered for prolong if a lot of demand exists.
[Maintainer]: I know it's mentioned above, but just to be very clear this issue is NOT about IE11 (or other legacy browsers) support. The ship has sailed on that. This is only about whether there is value in proving a pre-build ES5 distribution to use with picky/older build systems that haven't caught up to ES6 yet.
The text was updated successfully, but these errors were encountered: