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

Preserve property order in environment editor #1497

Merged

Conversation

Projects
None yet
2 participants
@develohpanda
Copy link
Contributor

commented May 12, 2019

Closes #1046.

image

Added an option to keep property order by consuming json-order. This is opt-in, thus disabled by default. This behavior is available to both the workspace environment editor, and the request group override environment editor.

Field names cannot begin by $ or contain a . (as part of the nedb documentation).

As a result, the map generation uses & as the prefix and ~| as the separator. These two options are parameters in the parse and stringify methods. An example map is shown below.

const map = {
  "&": ["a"],
  "&~|a": ["z", "y"],
  "&~|a~|z": ["b"],
  "&~|a~|y~|2": ["d", "c"]
}
@develohpanda

This comment has been minimized.

Copy link
Contributor Author

commented May 12, 2019

I'm not sure I added the dependency correctly - should json-order be under packedDependencies or dependencies?

@develohpanda develohpanda changed the title 1046 - sort environment variables Preserve property order in environment editor May 12, 2019

@gschier

This comment has been minimized.

Copy link
Collaborator

commented May 15, 2019

I'm not sure I added the dependency correctly - should json-order be under packedDependencies or dependencies?

It should be in both. packedDependencies is a special insomnia-specific property that the build script uses to figure out whether the dependency should be bundled in the Webpack output or left as an external node module. In general, most should be bundled, but some modules don't work like that so need to be left alone.

@develohpanda

This comment has been minimized.

Copy link
Contributor Author

commented May 16, 2019

Hmmm - I'm not sure why the CI builds are failing. I added to the packed dep but it still failed. Surely I've missed a step somewhere while adding the package 🤔

Works locally:
image

@gschier

This comment has been minimized.

Copy link
Collaborator

commented May 21, 2019

Ah, @develohpanda it's because you need to add a Flow definition to the flow-typed folder. I wish there were a way to have a better error message when that happens.

Here's a good example of one to take a look at:

https://github.com/getinsomnia/insomnia/blob/639d0cc33b0d455f43f449da1cbbe42dffa443cc/packages/insomnia-app/flow-typed/mkdirp.js

@gschier
Copy link
Collaborator

left a comment

PR looking great! Just a couple small comments and then one about whether or not we should have the checkbox to maintain order.

Show resolved Hide resolved packages/insomnia-app/app/models/environment.js Outdated
Show resolved Hide resolved packages/insomnia-app/app/models/request-group.js Outdated
Show resolved Hide resolved packages/insomnia-app/app/ui/components/editors/environment-editor.js Outdated
Show resolved Hide resolved packages/insomnia-app/app/ui/components/editors/environment-editor.js Outdated
@@ -30,10 +30,20 @@ class EnvironmentEditModal extends PureComponent {
return;
}

const environment = this._envEditor.getValue();
let patch;

This comment has been minimized.

Copy link
@gschier

gschier May 21, 2019

Collaborator

Curious why you included the try/catch when there wasn't one before. Did you find an error case that wasn't handled?

@@ -342,9 +343,13 @@ class WorkspaceEnvironmentsEditModal extends React.PureComponent<Props, State> {
return;
}

let data;
let patch;
try {

This comment has been minimized.

Copy link
@gschier

gschier May 21, 2019

Collaborator

Oh I see. Same thing here. I just checked the getValue function and didn't realize it was doing JSON parsing. Makes sense now 👍

Show resolved Hide resolved ...somnia-app/app/ui/components/modals/workspace-environments-edit-modal.js
Show resolved Hide resolved packages/insomnia-app/app/ui/css/layout/base.less Outdated

develohpanda added some commits May 23, 2019

@develohpanda

This comment has been minimized.

Copy link
Contributor Author

commented May 27, 2019

@gschier ready for review again :)

@gschier
Copy link
Collaborator

left a comment

Awesome! Just gave it a spin locally and it works great 😄

@gschier gschier merged commit 24a2e75 into getinsomnia:develop May 29, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@develohpanda develohpanda deleted the develohpanda:feature/1046-sort-env-variables branch May 31, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.