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

Add Dynamic Import when avalible. #121

Closed
masters3d opened this issue Nov 3, 2017 · 17 comments
Closed

Add Dynamic Import when avalible. #121

masters3d opened this issue Nov 3, 2017 · 17 comments
Labels
chore 🔧 Meta related task such as build, test, linting, maintainers.json etc.

Comments

@masters3d
Copy link
Contributor

Dynamic import would allow us to be able to import the exercise.examplebut it that doesn't exist then we could just import exercise. This would help in development.
https://github.com/tc39/proposal-dynamic-import

This would be similar to what we already do in the Objective-C track

#if __has_include("AllergiesExample.h")
# import "AllergiesExample.h"
# else
# import "Allergies.h"
#endif
@masters3d
Copy link
Contributor Author

@thedevelopnik
Copy link
Contributor

thedevelopnik commented Dec 12, 2017

So this will require a refactor of all existing exercises? and probably guidance for future exercises on using this?

@masters3d
Copy link
Contributor Author

Yeah, this would be a fund little project. It would mean that people can develop contributions and still be able to run using the normal yarn/npm process.

@thedevelopnik
Copy link
Contributor

Cool, I'm going on vacation on the 20th and will have some free time to actually do fun little projects, I can tackle this over the break.

@masters3d
Copy link
Contributor Author

Nice. Go for it!

@thedevelopnik
Copy link
Contributor

Starting on it now.

@thedevelopnik
Copy link
Contributor

@masters3d digging into this and I'm having some issues implementing it.

Started at the top level import, changing:

import Acronym from './acronym'

to

const Acronym = import('./acronym.example') || import('./acronym')

but now in the test code TS shows the error Property 'parse' does not exist on type 'Promise<typeof ".../exercism/typescript/exercises/acronym/acronym.example">'

It's a Promise that has yet to resolve.

That's why in the example you linked to above in the 2.4 release notes they show it in an async/await situation.

So if we move away from the top level and put it inside the test, we run into an issue because describe and it aren't async, so we can't await the conditional import statement ||.

We end up with this:

describe('Acronym are produced from', () => {
  it('title cased phrases', () => {
    return async function getModule() {
      const Acronym = await import('./acronym.example') || await import('./acronym')
      expect(Acronym.parse('Portable Network Graphics')).toEqual('PNG')
    }
  })
}

but now TS shows this error: Property 'parse' does not exist on type 'typeof "/Users/davesudia/exercism/typescript/exercises/acronym/acronym.example"'.

It's now a resolved promise but now I'm getting the TS error Property 'parse' does not exist on type 'typeof "/Users/davesudia/exercism/typescript/exercises/acronym/acronym.example"'.

Wat? If I comment out the const Acronym bit and add the regular import Acronym from './acronym.example' it's fine. So it seems to be reading the output from import differently than from await import()

Putting this here to see if there's something obvious I'm missing... I'll push up a branch so you can see the code as well.

@thedevelopnik
Copy link
Contributor

#129

@thedevelopnik
Copy link
Contributor

TypeScript will not compile if one of the imports isn't available/doesn't exist. This feature (in TS at least) seems to be more for memory/bundle optimization than for contingent importing. Closing this issue.

@masters3d
Copy link
Contributor Author

thanks for trying @thedevelopnik ! lets leave it open. i’d like to give it a try.

@masters3d
Copy link
Contributor Author

masters3d commented Jan 1, 2018

the next thing to try would be the .then .catch syntax
https://github.com/basarat/typescript-book/blob/master/docs/project/dynamic-import-expressions.md

@masters3d
Copy link
Contributor Author

Cant have top level await. May need to drop down to node for this feature
tc39/proposal-async-await#9 (comment)
https://gist.github.com/Rich-Harris/0b6f317657f5167663b493c722647221

@masters3d
Copy link
Contributor Author

masters3d commented Jan 7, 2018

This works but it is not ideal:

const fileExists = require('file-exists');

describe('Hello World', async () => {
  async function loadModule(){
    let example = ''
    if (fileExists.sync('hello-world.example.ts')) {
      example = '.example'
    }
    return await import('./hello-world' + example)
    .then(module => module.default)
  }

  it('says hello world with no name', async () => {
    expect(HelloWorld.hello()).toEqual('Hello, World!')
  })

  it('says hello to bob', async () => {
    expect(HelloWorld.hello('Bob')).toEqual('Hello, Bob!')
  })

  it('says hello to sally', async () => {
    expect(HelloWorld.hello('Sally')).toEqual('Hello, Sally!')
  })

  const HelloWorld = await loadModule()
})

@masters3d
Copy link
Contributor Author

The next step would be to warp this into a library or function so that we could say.

const HelloWorld  = importElse('./hello-world.example', './hello-world')

@masters3d
Copy link
Contributor Author

On the flip-side, We could export conditionally on the user facing stub file (not example).

https://stackoverflow.com/a/43591970/3705470

const exportedAPI = shouldUseMock ? mockAPI : realAPI
export default exportedAPI

@masters3d
Copy link
Contributor Author

2.9 seems to be the version that we want fo this:
microsoft/TypeScript#14844

@SleeplessByte SleeplessByte added the chore 🔧 Meta related task such as build, test, linting, maintainers.json etc. label Apr 12, 2019
@SleeplessByte
Copy link
Member

We have top-level track package.json tooling now that works without dynamic import.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore 🔧 Meta related task such as build, test, linting, maintainers.json etc.
Projects
None yet
Development

No branches or pull requests

3 participants