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

fix: marko/compiler deprecation warning #21

Closed
wants to merge 2 commits into from

Conversation

snyamathi
Copy link

@snyamathi snyamathi commented Sep 27, 2022

Description

This PR switches from using marko/compiler to @marko/compiler, resolving the deprecation warning (below). There's a few things that I don't like about this PR, but lack the context to resolve quickly. I might need some help getting this over the finish line (cc @DylanPiercey).

However, the changes do work for me locally 🎉

⚠️ There is a reference to Marko 4 vs 5, so this might effectively be dropping support for Marko 4. I'm not sure of the support status of Marko 4 so this might need to be delayed until support for Marko 4 is dropped and/or major version bump this lib.

Motivation and Context

WARNING!!
Using `marko/compiler` has been deprecated, please upgrade to the `@marko/compiler` module.
  at node:internal/modules/cjs/loader:1105:14

Screenshots (if appropriate):

Checklist:

  • I have updated/added documentation affected by my changes.
  • I have added tests to cover my changes.

@linux-foundation-easycla
Copy link

CLA Missing ID CLA Not Signed

const result = compiler[
(browser || (config as any).browser) &&
compiler.compileForBrowser /** Only Marko 4 supports compileForBrowser, otherwise use compile */
? "compileForBrowser"
: "compile"
](src, filename, MARKO_OPTIONS);

const result = compiler.compileSync(src, filename, MARKO_OPTIONS);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is definitely the biggest unknown; there is no compileForBrowser in Marko 5

let code = typeof result === "string" ? result : result.code; // Marko 3 does not support sourceOnly: false
let map = result.map;
let map: any = result.map;
Copy link
Author

@snyamathi snyamathi Sep 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

concatMap.add(filename, code, map);

yields

Argument of type 'SourceMap' is not assignable to parameter of type 'string | RawSourceMap | undefined'.
  Type 'SourceMap' is not assignable to type 'RawSourceMap'.
    Types of property 'version' are incompatible.
      Type 'number' is not assignable to type 'string'.ts(2345)

@DylanPiercey
Copy link
Collaborator

@snyamathi thanks for creating the PR!

I'd rather keep it so that this module can support both Marko 4 and 5.
I will look into what's needed to do that, while getting rid of this deprecation message.

@snyamathi
Copy link
Author

Absolutely - I was going to just raise an issue, but felt bad at least not attempting to see if I could do it myself.

Per tradition, I'll continue to ignore the warning for the time being 😆

Thanks for all the work you (all) do with Marko, much appreciated. Feel free to close this if you don't need it open for tracking.

@DylanPiercey
Copy link
Collaborator

@snyamathi I just did some project cleanup and improvements including this change over in #22.
This had been released as @marko/[email protected]. Let me know if you have any issues!

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 this pull request may close these issues.

2 participants