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

getContentfulEnvironment.js #33

Closed
bsgreenb opened this issue May 2, 2020 · 11 comments · Fixed by #110
Closed

getContentfulEnvironment.js #33

bsgreenb opened this issue May 2, 2020 · 11 comments · Fixed by #110
Labels

Comments

@bsgreenb
Copy link

bsgreenb commented May 2, 2020

Why is it js instead of ts?

@stevenpetryk
Copy link
Contributor

I didn’t want ts-node to be a dependency of the project when it’s not necessary. You’re welcome to make it TypeScript if you prefer, and compile it to JS.

@bsgreenb
Copy link
Author

bsgreenb commented May 4, 2020

@stevenpetryk
Copy link
Contributor

Yep! But it’s a devDependency—users don’t download it just by using this library.

@bsgreenb
Copy link
Author

bsgreenb commented May 4, 2020

@stevenpetryk Was about to correct myself. Also I was posting to the wrong repo anyways! I realized this file is a relic of my attempting to use this before. With Gatsby I've used graphql-codegen, because (I think) this doesn't work with Gatsby+Contentful setup

@G-Rath
Copy link
Contributor

G-Rath commented Jan 17, 2022

@stevenpetryk would you be open to supporting using TypeScript with ts-node as an optional peer dependency?

I've implemented this in a few projects like jest & postcss, and would be happy to do it again here if you're happy with that :)

@stevenpetryk
Copy link
Contributor

Hi @G-Rath, I don't work at Intercom anymore and so I cannot make changes to this project. I would recommend tagging a recent committer.

@GabrielAnca
Copy link
Contributor

Hi @G-Rath 👋🏻 I'd be interested in seeing how that would work! I'd be open to that as long as we keep backwards compatibility 😃 Would you mind going in a bit more detail on how your proposal would work? Or maybe if it's easier you can create a PR with your proposal and tag me there!

@G-Rath
Copy link
Contributor

G-Rath commented Jan 19, 2022

@GabrielAnca no problem - I'll look at doing a PR over the weekend.

The general way it's implemented is by doing a require for ts-node and if that's successful enabling it as a register before requiring the config file, then disabling it afterwards.

The implementation generally ends up looking something like this:

let registerer;

export const loadTSConfig = (configPath: string) => {
  try {
    // eslint-disable-next-line @typescript-eslint/no-var-requires
    registerer ||= require('ts-node').register() as Service;
  } catch (e: any) {
    if (e.code === 'MODULE_NOT_FOUND') {
      throw new Error(
        `'ts-node' is required for TypeScript configuration files. Make sure it is installed\nError: ${e.message}`
      );
    }

    throw e;
  }

  registerer.enabled(true);

  // eslint-disable-next-line @typescript-eslint/no-var-requires
  const configObject = interopRequireDefault(require(configPath)).default;

  registerer.enabled(false);

  return configObject;
};

There's some variation depending on how the config loading is actually done which I've not looked at yet e.g. for postcss that was using a config loading library (cosmic-config, iirc) so it was implemented like above but as a loader (which the config library handled determining when to actually try and use that loader), whereas for jest we check if the config file we've found ends with .ts (and consider jest.config.ts as a valid config when searching on-disk) and call the config before disabling the registerer if it's a function so that dynamic imports still work.

(tbh it's getting to the point where I've been considering publishing a package for handling it, because of how similar the core code is 🙈)

@G-Rath
Copy link
Contributor

G-Rath commented Jan 19, 2022

Looking over the codebase, I think it should be a matter of wrapping these lines in some logic mixed with the above.

What I can't see is an interface that defines what the config should look like as this is type any - @GabrielAnca is that something you could help with creating? (as I expect you probably know all the supported options and their types better than me at this point)

@G-Rath
Copy link
Contributor

G-Rath commented Jan 21, 2022

@GabrielAnca I just realised that this is using TypeScript 3 - would you be interested in me upgrading it to use v4? (and prettier?)

@GabrielAnca
Copy link
Contributor

🎉 This issue has been resolved in version 3.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging a pull request may close this issue.

4 participants