Skip to content
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

ESM module build? #151

Open
bseib opened this issue May 7, 2022 · 10 comments · May be fixed by #158
Open

ESM module build? #151

bseib opened this issue May 7, 2022 · 10 comments · May be fixed by #158

Comments

@bseib
Copy link

bseib commented May 7, 2022

Have you considered building the lib so that it is ESM compatible? I.e. for modern tooling that uses import statements rather than require(), and can do tree-shaking optimizations, etc. Here's an example library project setup using esbuild.

@bseib
Copy link
Author

bseib commented Feb 2, 2023

Here is my fork as an example of how to get it to build an ESM compatible build.

https://github.com/Gentomi/velocity.js

The changes needed: master...Gentomi:velocity.js:master

@shepherdwind
Copy link
Owner

shepherdwind commented Feb 4, 2023

I've recently been rewriting my code using typescript. I'll post an ESM package when I'm done. Thanks for you code example.

@shepherdwind shepherdwind linked a pull request Feb 14, 2023 that will close this issue
@shepherdwind
Copy link
Owner

@bseib now I published new beta version 2.1.0-beta, Can you help me to check if this version is work ok with esm module?

@bseib
Copy link
Author

bseib commented Feb 14, 2023

@bseib now I published new beta version 2.1.0-beta, Can you help me to check if this version is work ok with esm module?

Hey awesome @shepherdwind. I should have a chance to look at it this weekend.

@bseib
Copy link
Author

bseib commented Feb 20, 2023

Hey @shepherdwind , do you have any plans to release your BETA branch to npm? That would make testing a lot simpler -- I would just update my package.json to point at your new BETA version.

@shepherdwind
Copy link
Owner

Hey @shepherdwind , do you have any plans to release your BETA branch to npm? That would make testing a lot simpler -- I would just update my package.json to point at your new BETA version.

https://www.npmjs.com/package/velocityjs/v/2.1.0-beta this beta version is the new one, you can just try this. I had published it a week ago.

@bseib
Copy link
Author

bseib commented Feb 21, 2023

https://www.npmjs.com/package/velocityjs/v/2.1.0-beta this beta version is the new one, you can just try this. I had published it a week ago.

Ah, thank you, I missed that somehow.

Things look pretty good. I have one snippet of code left to fix, and I haven't dug in yet to see what's going on. My function was parsing a velocity template, then looking through the abstract syntax tree to get all the parameter names that were found. I need to figure out the typescript equivalent for x.id that worked in javascript.

const parseOutParamNames: ((template: string) => string[]) = (template: string) => {
      const astArray = (vjs.parse(template)) as Array<VELOCITY_AST>
      return astArray.filter((x) => {
        return typeof x === 'object'
      }).map((x) => {
        return x.id
      })
    }

I ought to be able to make a new function that is cleaner with respect to types now that the underlying code is also in typescript.

@bseib
Copy link
Author

bseib commented Feb 25, 2023

@shepherdwind I think VELOCITY_AST needs to be exported at the top level. That is the type returned when calling parse, and the caller will need that type to do anything useful. I tried to import it directly, i.e.,

import {VELOCITY_AST} from "velocityjs/dist/types/type"

However that lead to other compilation issues.

I ought to be able to simply do:

import {parse, render, VELOCITY_AST} from 'velocityjs'

@bseib
Copy link
Author

bseib commented Feb 25, 2023

@shepherdwind The second issue I'm seeing is a runtime error:

image

The sources are not in the distribution, but it looks like this may be referring to here:

export class BlockCommand extends Compile {

Maybe there is no default constructor up the chain of classes? Not sure what's going on there...

@shepherdwind
Copy link
Owner

@shepherdwind The second issue I'm seeing is a runtime error:

image

The sources are not in the distribution, but it looks like this may be referring to here:

export class BlockCommand extends Compile {

Maybe there is no default constructor up the chain of classes? Not sure what's going on there...

Can you provide me you code? I just try this in node.

It is better to have a project that can reproduce the whole scene.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants