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

Allow dynamically defining profiles on esm configs #2009

Closed
notaphplover opened this issue Apr 24, 2022 · 11 comments · Fixed by #2384
Closed

Allow dynamically defining profiles on esm configs #2009

notaphplover opened this issue Apr 24, 2022 · 11 comments · Fixed by #2384
Labels
good first issue Good for newcomers ✅ accepted The core team has agreed that it is a good idea to fix this ⚡ enhancement Request for new functionality

Comments

@notaphplover
Copy link
Contributor

🤔 What's the problem you're trying to solve?

As developer I would love to dinamycally define profiles on cucumber es module configs.

Actually, the only way to define profiles is exporting the configuration associated to the profile:

cucumberConfig.js

/** @type {import("@cucumber/cucumber/lib/configuration").IConfiguration} */
const profile1 = getConfiguration('profile1');

/** @type {import("@cucumber/cucumber/lib/configuration").IConfiguration} */
const profile2 = getConfiguration('profile2');

/** @type {import("@cucumber/cucumber/lib/configuration").IConfiguration} */
const defaultCucumberConfig = getConfiguration();

export default defaultCucumberConfig;

export { profile1, profile2 };

If I have several dinamycally generated configs, I can't figure out any way to export them in an esm with the current cucumber-js profile configuration.

✨ What's your proposed solution?

There's not a single and best way to achieve this imho. Maybe a default export should be mandatory and it would contain a config or an object with profiles, being default the default one:

cucumberConfig.js

/** @type {import("@cucumber/cucumber/lib/configuration").IConfiguration} */
const profile1 = getConfiguration('profile1');

/** @type {import("@cucumber/cucumber/lib/configuration").IConfiguration} */
const profile2 = getConfiguration('profile2');

/** @type {import("@cucumber/cucumber/lib/configuration").IConfiguration} */
const defaultCucumberConfig = getConfiguration();

const globalConfig = {
  default: defaultCucumberConfig,
  profile1,
  profile2,
}

export default globalConfig;

I'm aware this is a breaking change, cucumber-js could temporaly (until next major) support both syntaxes for a non breaking approach.

⛏ Have you considered any alternatives or workarounds?

Yeah, I could use a common js module but if I'm not mistaken I think esm should not be a second class citizien.

📚 Any additional context?

Consider ECMAScript spec as reference


This text was originally generated from a template, then edited by hand. You can modify the template here.

@davidjgoss
Copy link
Contributor

Sorry, I'm not clear on exactly what the problem is here or how it's specific to ESM.

Could you post a concrete example of something you can do with CommonJS but isn't possible with ESM?

@davidjgoss davidjgoss added the 🍼 incomplete Blocked until more information is provided label Apr 24, 2022
@notaphplover
Copy link
Contributor Author

Sure ^_^:

CommonJS scenario:

A CommonJS module can be this one:

/** @type {Object.<string, import("@cucumber/cucumber/lib/configuration").IConfiguration>} */
const globalConfig = getGlobalConfig();

module.exports = globalConfig

Keep in mind this globalConfig is an object with string keys (the profile names) and cucumber configs values. In CommonJS I don't need to declare, one by one, the configs I want to export, I can simply set the object to module.exports and cucumber js will accept it.

ESM scenario

There is no way to do this in ESM, I cannot export an object, I can only export an object associated to a key or to the default export:

  • I could export default globalConfig, but cucumber only sees a single invalid default config which is the whole object: an object with string keys (the profile names) and cucumber configs values.

To export the object I have to:

export default globalConfig.default;
export const { profile1, profile2, profile3 } = globalConfig;

But I cannot write a code that exports as many profiles are in globalConfig due to ESM syntax, any export requires the creation of a new variable

@davidjgoss
Copy link
Contributor

Thanks @notaphplover, that makes sense now. I'll follow up here soon.

@notaphplover
Copy link
Contributor Author

notaphplover commented May 19, 2022

Hi @davidjgoss, may i do something for you at this point? I see the incomplete label on the issue, if you need any additional clarification feel free to reach me 😃

@davidjgoss
Copy link
Contributor

Sorry for the late reply @notaphplover. I'm keen to support this but also to maintain the existing exports pattern as I think it's ergonomic for most users. A bit similar to your idea, my thought would be a special case name like _profiles which you could export and would be treated as a map of profiles.

@davidjgoss
Copy link
Contributor

and/or, your default export could be a function that returns (a promise for) a map of profiles?

@davidjgoss davidjgoss added ✅ accepted The core team has agreed that it is a good idea to fix this ⚡ enhancement Request for new functionality and removed 🍼 incomplete Blocked until more information is provided labels May 24, 2022
@notaphplover
Copy link
Contributor Author

notaphplover commented May 24, 2022

Hi @davidjgoss, nothing to say sorry for. Thank you so much for the effort!

I would prefer the default export exporting a function that returns a promise for a map of profiles. _profiles is also a good choice, I don't really have a strong opinion here.

If I had to be picky, thinking in developer experience, I think it's easier to understand a config module which can export an object or a function rather than thinking there's a special _profiles export for doing something which seems to be redundant (of course it's not but I'm thinking in someone who never thought in esm modules nor watched this issue).

I'm happy to contribute to the project following your best practices if you want

@davidjgoss davidjgoss changed the title Allow dinamycally defining profiles on esm configs Allow dynamically defining profiles on esm configs Jun 19, 2022
@xenbartolokath
Copy link

Hello! Any updates on this?

@notaphplover
Copy link
Contributor Author

Hi @xenbartolokath , none so far, if you are interested in this feature I would suggest you to thumbs up the issue 😃

@davidjgoss
Copy link
Contributor

davidjgoss commented Dec 12, 2023

Thanks for showing your interest folks. The core team will get to this but I can't promise when.

Meanwhile, if anyone is up for contributing a PR, we'd more than welcome that, and the relevant code is here https://github.com/cucumber/cucumber-js/blob/main/src/configuration/from_file.ts. I think we have a consensus around a default export of an async function that resolves to the profiles.

@davidjgoss davidjgoss added the good first issue Good for newcomers label Dec 12, 2023
@notaphplover
Copy link
Contributor Author

Hey @davidjgoss, sry for the late reply, I just saw it. All right, I'll go for it 😃, I think I should be able to deliver a PR in less than a week, depending on the spare time I have

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers ✅ accepted The core team has agreed that it is a good idea to fix this ⚡ enhancement Request for new functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants