-
Notifications
You must be signed in to change notification settings - Fork 135
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
Remove assert import in favour of webpack #1254
Comments
Did you try to add the suggested plugin? We use I agree with you that these assertions are ugly and should be unnecessary, but they seem to be a quirk of the present day JS/TS world. If we removed them, other things would break 🫤 🤕 |
I found no way how to add it successfully into the webpack config (there are ways to point to a custom webpack config and then add babel as a module). Most plugins are designed to format the code in the source folder, not in the node_modules one. With the step from the script I was able to resolve it, but I honestly don't like the solution. According to the docs assertion help the compiler, but since the imported data aren't that huge I would suggest we remove them |
@mirceanis I tried different ways out. When modifying the code with
it works. But in my case I am working in a monorepo and the backend services will throw an error:
One suggestion would be to place the content inside a variable and then import it. In this case we already have it as a object and do not need any assert method. In case I find a solution with webpack like the existing one with babel this could also work without changing anything to the code |
I'm having the same issue with an ionic/angular app. Your workaround of deleting the assert portion of those statements worked, so thanks. Even though this didn't work, this is where I got with the babel.config.js file: module.exports = {
presets: [
[
'@babel/preset-typescript',
'@babel/preset-env',
{
'targets': {
'node': 'current'
}
}
]
],
plugins: [
[
'@babel/plugin-syntax-import-attributes',
{ 'deprecatedAssertSyntax': true }
],
'@babel/plugin-syntax-import-assertions'
]
}; It doesn't appear that my build process looked at this file, so maybe it needs to go into @Angular-devkit. Anyway, maybe this will trigger a thought on your end. |
In the meantime I used The best solution in my mind would be to move the json object into a ts file and export it as a variable. With this approach we can include it in all frameworks independent of some typescript features. To break no other third party dependents that rely on the json file, I would suggest to ADD the ts file also in the folder and update the core libs of veramo where we see the errors right now. Of course we need to check if the two files are in sync all the time. (this could be guaranteed with some kind of unit test before we publish it) |
@mirceanis I found another solution that won't break current functionality:
This approach will work in all environments since it's a native import without any new functions like typed imports that are not fully supported in the different bundlers. It will also not break current implementation, so it's more a feature than a breaking change :) When all bundlers are supporting this, we can go back to the assert approach for optimization. The only downside is an increased package by 254KB since we included the same object twice (as json and as js object). But in my opinion this tradeoff is worth it. We could even minify the object and the json to remove white space. |
Plugin schemas are (mostly) auto-generated. This would be a breaking change as it affects the core logic. Next, we'd have to replace the imports/exports of the JSON-LD contexts to similarly load them from *.js files instead of *.json(ld), although, in this case we need to ensure that contexts can still be loaded asynchronously from file URLs as the default ones are quite heavy and might not be needed by everyone. |
I am not a fan of a breaking change... so maybe in the v5 we can add the extra file and maybe in v6 we can remove the json approach? Since we are using the assert approach it's not possible to mark the json function as deprecated like you can do it with functions, classes, etc in JS or TS :( |
Fixes decentralized-identity#1254 Signed-off-by: Mirko Mollik <mirko.mollik@fit.fraunhofer.de>
Fixes decentralized-identity#1254 Signed-off-by: Mirko Mollik <mirko.mollik@fit.fraunhofer.de>
fixes #1254 BREAKING CHANGE: The `plugin.schema.json` files are now generated as `plugin.schema.ts`.
Bug severity
4
Describe the bug
When integrating some basic plugins for veramo, I get the following error:
Since I am using Angular that is based on webpack I found no way to deal with this problem. Adding a custom webpack and setting the babel loader with the mentioned plugin does not seem to work:
When I manually removed the
assert { type: 'json' };
from all the files it ran without the errors and I could not see any side effects.Right now I am using a script that runs after the installation of all packages and removes the assertions:
So my question is can we remove the assert feature in favour to be webpack compatible?
To Reproduce
Steps to reproduce the behaviour:
Add the veramo agent to an angular application with the
did-jwt-vc
pluginObserved behaviour
Having this feature in the core libraries makes it impossible to use them in non babel environments.
The text was updated successfully, but these errors were encountered: