Skip to content

Conversation

@hacksparrow
Copy link
Contributor

Add local API Explorer

Checklist

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

// openApiSpecUrl,
// );
// writeFileSync(explorerIndexFilePath, templateContent);
app.static(explorerPath, explorerLocalPath);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bajtos app.static seems to be overriding the ExplorerController . Any idea what's going on? Or did I misunderstand something?

.expect(content);
});

it('gives precedence to API controllers over static assets', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bajtos here is the test which catches the problem. Calling restApp.static('/html', root); after restApp.controller(DummyController); passes the test. That means, something's wrong in the order of registration /look up.

@hacksparrow hacksparrow self-assigned this Nov 8, 2018
@raymondfeng
Copy link
Contributor

@hacksparrow Please rebase this PR off master.

@hacksparrow hacksparrow force-pushed the local-api-explorer branch 2 times, most recently from e6b93aa to f45e4e3 Compare November 8, 2018 18:19
@hacksparrow
Copy link
Contributor Author

hacksparrow commented Nov 8, 2018

Notes on the current state of the PR

  • Apps can be configured to enable the local API Explorer by passing these two config options: enableExplorer and explorerPath. Eg:
  options.enableExplorer = true;
  options.explorerPath = '/explorer1';
  • On making a GET request to the root of the API Explorer as defined in explorerPath, we intent to serve a customized index.html file. However due to a current limitation, the file is not being served as text/html. Raymond is working on this issue - https://github.com/strongloop/loopback-next/tree/honor-response-content-type.

  • What do we do with the currently baked-in router for /explorer?

    • Do we remove it altogether?
    • What do we do if user set explorerPath to /explorer?

@bajtos
Copy link
Member

bajtos commented Nov 9, 2018

Apps can be configured to enable the local API Explorer by passing these two config options: enableExplorer and explorerPath.

I would personally prefer a slightly different configuration:

  • To enable local API explorer, apps have to add ExplorerComponent.
  • To disable local API explorer, apps should skip registration of ExplorerComponent.
    That way we can keep ExplorerComponent simpler. However, this solution may not be feasible depending on what we do with the built-in redirect to swagger-ui.

What do we do with the currently baked-in router for /explorer?

  • Do we remove it altogether?
  • What do we do if user set explorerPath to /explorer?

I feel that removing this feature would be a breaking change that requires semver-major. Are we ok with publishing @loopback/[email protected] just a month after v4 was released?

IMO, a better solution is to introduce a configuration flag allowing applications to disable this built-in redirect. When the Explorer component is mounted, it can set this flag to true to ensure the built-in redirect is disabled.

@bajtos
Copy link
Member

bajtos commented Nov 12, 2018

I opened a new PR inspired by this work, see #2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants