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

Issue #48: changed from commonjs to es6 modules #49

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

austen-wade
Copy link

@austen-wade austen-wade commented Oct 20, 2022

#48

I have tested this change on two separate ui.frontend repositories and they seem to work.
I configured the repositories as follows:

  • repo 1 had es6 modules system
  • repo 2 had commonjs modules system (what the archetype generates today)

I did not encounter any issue with either build.

Also, yargs had to be updated itself to latest for esmodules support. The api has slightly changed as the imported item is no longer a singleton.

@stefanseifert
Copy link
Member

i would like to switch our frontend modules to "type=module" as well.

i tried your branch, but failed to execute it on windows - seems to be a windows-specific path problem. when executing i got:

[ERROR] [nodejs] Error [ERR_UNSUPPORTED_ESM_URL_SCHEME]: Only URLs with a scheme in: file, data are supported by the default ESM loader. On Windows, absolute paths must be valid file:// URLs. Received protocol 'c:'
[INFO] [nodejs]     at new NodeError (node:internal/errors:372:5)
[INFO] [nodejs]     at throwIfUnsupportedURLScheme (node:internal/modules/esm/resolve:1120:11)
[INFO] [nodejs]     at defaultResolve (node:internal/modules/esm/resolve:1200:3)
[INFO] [nodejs]     at ESMLoader.resolve (node:internal/modules/esm/loader:580:30)
[INFO] [nodejs]     at ESMLoader.getModuleJob (node:internal/modules/esm/loader:294:18)
[INFO] [nodejs]     at ESMLoader.import (node:internal/modules/esm/loader:380:22)
[INFO] [nodejs]     at importModuleDynamically (node:internal/modules/esm/translators:106:35)
[INFO] [nodejs]     at importModuleDynamicallyCallback (node:internal/process/esm_loader:35:14)
[INFO] [nodejs]     at file:///C:/temp/betacloudProject/frontend/node_modules/aem-clientlib-generator/bin/clientlib-cli.js:48:1
[INFO] [nodejs]     at ModuleJob.run (node:internal/modules/esm/module_job:198:25) {
[ERROR] [nodejs]   code: 'ERR_UNSUPPORTED_ESM_URL_SCHEME'

seems to work fine on linux!

@mbehzad
Copy link
Contributor

mbehzad commented Apr 12, 2023

Hi @austen-wade ,

  • "file:///" needs to be added to configPath. Otherwise when importing, node wouldn't be able to distinguish between a windows path and an url.
  • process.env.npm_package_version will be the user's package.json's version who is calling clientlib via an npm script. but it was meant to show the aem-clientlib-generator's version in the cli.
  • you mentioned updating the yargs dependency but I couldn't see any changes to the package.json. [email protected] already supports esm. but as you mentioned its esm has a slightly different implementation compared to its cjs version. But what still is missing is the argv list (i.e. y = yargs(); -> y = yargs(hideBin(process.argv));
  • someone can also update the readme with an esm example (require(), module.exports , __dirname, etc)

cheers

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.

3 participants